Re: [Cppunit-devel] hierarchy sample bug...

[Bastiaan B. <bas...@li...>]

Baptiste Lepilleur wrote:

> > > >     Humm, just one thing that popup in my mind: who owns the fixture
> given
> > > > to the test caller ? That's may be the reason why it is done that
> way...
> > > >
> > > > I guess it had something to do with the automatic test registration
> thing,
> > > > which has been removed anyway.
> > >     No, it was done that way in Michael Feather version. And if I
> understood
> > > thing well, it's also done that way in junit.
> > >
> >
> > Not!
> > Please take a look at the JUnit code. JUnit doesn't have a TestCaller: it
> doesn't need one because it can use Java Reflection to lookup test methods
> and execute them.
> > Also, the only container for Tests is the TestSuite class, no Registries
> or other things.
> > It's simple and I like it.
>     They don't have a test caller class because the test caller is the test
> case itself. Here is a piece of code extracted from
> TestSuite.addTestMethod() (called for each public test method on
> construction):
>
>   if (isPublicTestMethod(m)) {
>    names.addElement(name);
>
>    Object[] args= new Object[]{name};
>    try {
>     addTest((Test)constructor.newInstance(args));            //  <=== a new
> instance is created which take the name of the method in the constructor !
>    } catch (Exception t) {
>     addTest(warning("Cannot instantiate test case: "+name));
>    }
>
> and in TestCase.runTest() that do what our test caller is doing:
> // fname is the name specified on construction of the test case, the name of
> the method to run!
>    runMethod= getClass().getMethod(fName, new Class[0]);
>     [...]
>   try {
>    runMethod.invoke(this, new Class[0]);
>   }
>
>     This exactly what our test caller do... In some way we have even more
> flexibility because we can decorate the test caller (adding running time
> constraint, expecting some exception...).
>

Hmm, yes, didn't look beyond the reflection stuff in TestCase. Don't know why
the Java version needs separate TestCase instances per Test though. Maybe
concurrency safety, since Java doesn't have the 'object ownership' problem at
least.

--8<--

>
>
> > Sorry to be blunt, but I think this sucks, because you need to be very
> careful to override makeFixture() in every subclass. Why can't the TestCase
> use a reference to itself instead?
>     Its depends how you make your code. I use IDE macro to generate and adds
> file to the project. I just check a box to say "this class is  a cppunit
> test case", and it adds everything I need to make the class a test case
> (static suite method, inheritance, includes...). This way I remove all the
> problem you have when using cut and paste to do that.
>
> > Ahem, the amended 'hierarchy' example seems like a perfectly valid example
> of how to use the same fixture for multiple TestCallers. Any objections?
>
>     There is a very bad thing about the way its done: you enforce the class
> owning the test caller to also own a reference to the test case. Any class
> you give the test caller to, must also own the instance of the test case
> that was used to create the test caller.

I don't see that. You only have to ensure the TestCase lives longer than the
TestCallers, right?

>
>     To me this is a big loss of flexibility. It makes it harder to use stuff
> such as decorator and break the "ownership" policy that is everywhere in the
> framework:
>     TestSuite owns all its tests (sub-suite, TestCaller, ...)
>     TestCaller owns all its TestCase
>     TestCase owns resources needed to run the test.
>     Each component of the hierarchy can be taken out of the system and works
> on its one. The TestCaller does not need the TestSuite to be able to run.
>
>     Usually, you don't build test case on the stack. You have a static
> suite() method in the test case returning a new TestSuite object. With the
> new ownership policy, you would have to subclass TestSuite so it could hold
> (and own) a reference on the TestCase instance.
>
>     To sum it up:
>     - the TestCaller would not be able to be a component running on its one
> (you could not pass it to the TestRunner because it need something "above"
> owning the test case).
>     - you provided a valid example of use of the test caller with a
> reference on the test case. Though there are underlying constraints to that
> use case (at suite level).
>     - this would break one of the constructor of TestSuiteBuilder since it
> could not use the generic TestSuite class, but would need to instantiate its
> owns subclassed TestSuite.
>
>     Sharing the reference on the test case does not make it easier to use,
> and split from the ownership policy that is in the framework. Using the
> standard template factory method pattern (makeFixture()) seems a lot easier
> to me and much more flexible.
>
> >
> > >
> > >     The only use I can see for the test caller with fixture by reference
> is if the Test Caller is stored by value in another class.
> > >
> > >     Things to do:
> > >         Update TestSuiteBuilder:
> > >             - add a addTestCaller() method which take a pointer on the
> fixture.
> > >             - add a constructor that take a pointer on a "makeFixture()"
> methods, to make
> > > subclassing easier.
>
> > Before you rush off to add makeFixture() methods, etc. Let us rethink this
> issue a bit. My personal feeling is that this registration stuff already is
> more complex than necessary, so adding more stuff is unlikely to be the
> proper solution. I prefer to steal more from JUnit :-)
>     I was not suggesting adding the makeFixture() method in the library.
> This is a user choice (just like having a registerTests() method). If the
> user make a test case that can be subclassed, he must design the class so it
> can be done. I was merely suggesting a way to do that.
>
>     Conclusion:
>         - adding the constructor with a pointer on the fixture add a lot of
> flexibility to test caller.
>         - enforcing the use of the constructor by reference remove a lot of
> flexility.
>
>     To do cppunit:
>         - remove TestRegistry.cpp/.h
>         - implement a TextTestRunner
>
>     To do for the sample:
>         - add a suite() method for each test case, which returns a
> dynamically allocated suite,
>         - add the suite of each test case to the TextTestRunner,
>         - run the TextTestRunner.
>     => this would make it similar to run suite using a graphic or text
> TestRunner, and would show how test case should be factored.
>

OK, thanks for the explanation. Unfortunately until the weekend I don't have
the time to sort it out: got an exam this friday. So this would stall the
release of 1.5.5 until at least somewhere next week.
If otherwise the current tree is OK (and it is, isn't it?), I wouldn't want to
deprive people from Win32 support until then, and release 1.5.5 as an ad
interim version. May give us some useful feedback from others too. If there're
are no nay-sayers, I'll put it on source forge tomorrow.

Bastiaan