Thread: [Cppunit-devel] Assertion, ASSERT_EQUAL & co...
Brought to you by:
blep
From: Baptiste L. <gai...@fr...> - 2001-10-05 12:30:47
|
I've been thinking about all our discussion about ASSERT_EQUAL and others assertion. I've come to the following conclusion: - CppUnit can not provide an ASSERT_EQUAL that satify everybody - CppUnit users need to create custome assertion (checking that two XML strings are identical for example ;-) ) 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 I won't discuss (1) now since it is long winded discussed, with little value at the end. I'll tackle (2) right now which offer the most value for us. 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) Introduce a new macro: #define CPPUNIT_SOURCELINE() CppUnit::SourceLine( __FILE__, __LINE__ ) Introduce helper methods in TestAssert to factor out code that throw exception for failure. (That last one is not well refined yet, but it would be something like: void fail( std::string message, SourceLine location = SourceLine() ); void failIf( bool shouldFail, std::string message, SourceLine location = SourceLine() ); void failNotEqual( std::string expected, std::string actual, SourceLine location = SourceLine(), std::string additionalMessage ="" ); // additionalMessage is a message that is append after the // 'expect ... but was ...'. I noticed that it is really helpful // to have a precise report indicating why both are not equals. See // the CPPUNITTEST_ASSERT_XML macro used to compare two XML string // for example. void failNotEqual( bool shouldFail, std::string expected, std::string actual, SourceLine location = SourceLine(), std::string additionalMessage ="" ); Note that some of those function probably exist in TestAssert). The next step would be to move assertion code outside of TestAssert (assertEquals...). TestAssert would provide service to report failed assertion, but would not implement those assertions (and therefore making no choice about how those assertion are implemented). This would leave the user with a very simple way to write new assertion MACRO. For example: MYASSSERT_XML_EQUAL( expected, actual ) \ checkXmlEqual( expected, actual, CPPUNIT_SOURCELINE() ); checkXmlEqual( std::string expected, std::string actual, SourceLine location ) { if ( expected != actual ) // Whatever you want there CppUnit::TestAssert( fail, expected, actual, location ); } I'll try to tackle the first part this evening (applying the IntroduceParameteObject refactoring). Does the change to TestAssert sound good for you ? Baptiste. --- Baptiste Lepilleur <gai...@fr...> http://gaiacrtn.free.fr/index.html Language: English, French |
From: Steve M. R. <ste...@vi...> - 2001-10-05 14:11:34
|
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. > 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. > > (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" ? > void failIf( bool shouldFail, > std::string message, > SourceLine location = SourceLine() ); This sounds like the replacement for the current "assertImplementation"? If so, why not call it simply "assert"? > 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). 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(): std::string should_equal_message( string expected, string actual, SourceLine location, string additional ) { return "Expected " + expected + " but got " ... } > Note that some of those function probably exist in TestAssert). > > The next step would be to move assertion code outside of TestAssert > (assertEquals...). TestAssert would provide service to report failed assertion, > but would not implement those assertions (and therefore making no choice about > how those assertion are implemented). > > This would leave the user with a very simple way to write new assertion > MACRO. For example: > > MYASSSERT_XML_EQUAL( expected, actual ) \ > checkXmlEqual( expected, actual, CPPUNIT_SOURCELINE() ); > > checkXmlEqual( std::string expected, > std::string actual, > SourceLine location ) > { > if ( expected != actual ) // Whatever you want there > CppUnit::TestAssert( fail, expected, actual, location ); > } > > I'll try to tackle the first part this evening (applying the > IntroduceParameteObject refactoring). > > Does the change to TestAssert sound good for you ? 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. -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. -- by Rocket to the Moon, by Airplane to the Rocket, by Taxi to the Airport, by Frontdoor to the Taxi, by throwing back the blanket and laying down the legs ... - They Might Be Giants |
From: Baptiste L. <gai...@fr...> - 2001-10-05 16:45:34
|
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 |
From: Baptiste L. <bl...@cl...> - 2001-10-05 22:33:21
|
Done. The SourceLine make the code much cleaner and compact (isValid() instead of magic comparison to UNKNOWNNUMBER ;-). I updated CPPUNITTEST_ASSERT_XML_EQUAL to use that mecanism and it works well. Also added a few other stuffs that make life easier. Well, I can't think right anymore, so it's time for me to call it a day. Good night, Baptiste. ----- Original Message ----- From: "Baptiste Lepilleur" <gai...@fr...> To: "Steve M. Robbins" <ste...@vi...> Cc: "Cpp Unit Develpment Mailing List" <cpp...@li...> Sent: Friday, October 05, 2001 6:45 PM Subject: Re: [Cppunit-devel] Assertion, ASSERT_EQUAL & co... > 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 l ittle > 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 > > _______________________________________________ > Cppunit-devel mailing list > Cpp...@li... > https://lists.sourceforge.net/lists/listinfo/cppunit-devel > |