Re: Making tests suck less (was Re: ECM++)
Ugo Cei wrote:
Il giorno 22/ott/04, alle 19:40, Reinhard Poetz ha scritto:
Have you considered using Easymock? IMHO its usage is more intuitive
than jMock.
(see BlockDeployer test cases)
After perusing the documentation and samples, I decided that I liked
jMock more, but I didn't try EasyMock in practice. If you are familiar
with it, you might try doing a version of the
ResourceExistsActionTestCase that I just refactored, so we can do a
comparison and decide which is better.
Surce, here we go:
public void testAct() throws Exception {
// create objects and controls
String src = don't care;
ResourceExistsAction action = new ResourceExistsAction();
MockControl resolverCtrl = MockControl.createControl(SourceResolver.class);
SourceResolver resolver = (SourceResolver) resolverCtrl.getMock();
MockControl sourceCtrl = MockControl.createControl(Source.class);
Source source = (Source) sourceCtrl.getMock();
MockControl parameterCtrl =
MockClassControl.createControl(Parameters.class);
Parameters parameters = (Parameters) parameterCtrl.getMock();
// record excpected method calls, return values and exceptions
source.exists();
sourceCtrl.setReturnValue(true, MockControl.ONE);
sourceCtrl.replay();
resolver.resolveURI(src);
resolverCtrl.setReturnValue(source, MockControl.ONE);
resolver.release(source);
resolverCtrl.replay();
parameters.getParameter(url, src);
parameterCtrl.setReturnValue(src);
parameterCtrl.replay();
// call method to be tested
Map result = action.act(null, resolver, new HashMap(), src, parameters);
// check result
assertSame(Test if resource exists, AbstractAction.EMPTY_MAP, result);
// check mock objects if methods have been called correctly
resolverCtrl.verify();
parameterCtrl.verify();
sourceCtrl.verify();
}
However, I suspect it's more a
matter of taste than anything else.
Probably yes. IMO the advantages of Easymock are
- you directly call methods on the objects - so refactoring is rather simple
because the TestMethods are updated too
- Easymock can also mock classes and not only interfaces
Ugo, how did you mock the Parameters object?
IMO the disadvantage is that it is more verbose than jMock.
Of course, if you rewrite our tests, it's your decision :-)
--
Reinhard
RE: Making tests suck less (was Re: ECM++)
Ugo Cei wrote: In the coming days, I plan to rewrite all tests depending on ExcaliburTestCase so that we can forget about it. Stay tuned. In addition, our tests test if it is possible to get a corresponding selector (e.g. for transformers etc.) and then the component to test from this selector. In order to make this work, you have to configure these things in the xtest file - now I think this is wrong. The test should simply consists of creating the object to test and then test it - without the overhead of looking up a selector etc. This is something we should remove also. Carsten
Re: Making tests suck less (was Re: ECM++)
Il giorno 25/ott/04, alle 08:58, Carsten Ziegeler ha scritto: Ugo Cei wrote: In the coming days, I plan to rewrite all tests depending on ExcaliburTestCase so that we can forget about it. Stay tuned. In addition, our tests test if it is possible to get a corresponding selector (e.g. for transformers etc.) and then the component to test from this selector. In order to make this work, you have to configure these things in the xtest file - now I think this is wrong. The test should simply consists of creating the object to test and then test it - without the overhead of looking up a selector etc. This is something we should remove also. +1. Unfortunately you did too good a job of removing dependencies on ExcaliburTestCase and took away much of the incentive for me to rewrite them all ;-). I'll have to review them again and decide whether we can keep them as they are now. Ugo -- Ugo Cei - http://beblogging.com/ smime.p7s Description: S/MIME cryptographic signature
RE: Making tests suck less (was Re: ECM++)
Ugo Cei wrote: Il giorno 25/ott/04, alle 08:58, Carsten Ziegeler ha scritto: Ugo Cei wrote: In the coming days, I plan to rewrite all tests depending on ExcaliburTestCase so that we can forget about it. Stay tuned. In addition, our tests test if it is possible to get a corresponding selector (e.g. for transformers etc.) and then the component to test from this selector. In order to make this work, you have to configure these things in the xtest file - now I think this is wrong. The test should simply consists of creating the object to test and then test it - without the overhead of looking up a selector etc. This is something we should remove also. +1. Unfortunately you did too good a job of removing dependencies on ExcaliburTestCase and took away much of the incentive for me to rewrite them all ;-). I'll have to review them again and decide whether we can keep them as they are now. :) Sorry about that :) Now, I think, we should remove the selector handling from the SitemapComponentTestCase and instead of configuring the component I want to test in the xtest file with an addition selector, simply write in our tests something like: Matcher m = this.createComponent(CLASS_NAME); where the createComponent method is defined in the SitemapComponenTestCase class. It creates the object, invokes all Avalon lifecycle interfaces and that's it. I wanted to change this tonight. Carsten
Re: Making tests suck less (was Re: ECM++)
Il giorno 25/ott/04, alle 08:17, Reinhard Poetz ha scritto: Probably yes. IMO the advantages of Easymock are - you directly call methods on the objects - so refactoring is rather simple because the TestMethods are updated too Interesting. This merits some more consideration. I also don't like very much jMock's long chains of calls and its using strings for method names, which makes refactoring harder. Ugo, how did you mock the Parameters object? I didn't. I created a real Parameters object (empty, by the way) and passed that to the methods under test. Being a little more than a map, what's the point mocking it? Ugo -- Ugo Cei - http://beblogging.com/ smime.p7s Description: S/MIME cryptographic signature
Re: Making tests suck less (was Re: ECM++)
Ugo Cei wrote: Il giorno 25/ott/04, alle 08:17, Reinhard Poetz ha scritto: Probably yes. IMO the advantages of Easymock are - you directly call methods on the objects - so refactoring is rather simple because the TestMethods are updated too Interesting. This merits some more consideration. I also don't like very much jMock's long chains of calls and its using strings for method names, which makes refactoring harder. ... and code completion works (which is the most valuable advantage IMHO) Ugo, how did you mock the Parameters object? I didn't. I created a real Parameters object (empty, by the way) and passed that to the methods under test. Being a little more than a map, what's the point mocking it? In this case, nothing ;-) But sometimes there are cases when you have to mock an object and not an interface and Easymock can build proxy objects of them too. -- Reinhard
Re: Making tests suck less (was Re: ECM++)
Il giorno 25/ott/04, alle 09:39, Reinhard Poetz ha scritto: But sometimes there are cases when you have to mock an object and not an interface and Easymock can build proxy objects of them too. Not true. You can mock classes with jMock too: http://jmock.org/cglib.html :-) Ugo -- Ugo Cei - http://beblogging.com/ smime.p7s Description: S/MIME cryptographic signature
Re: Making tests suck less (was Re: ECM++)
Ugo Cei wrote: Il giorno 25/ott/04, alle 09:39, Reinhard Poetz ha scritto: But sometimes there are cases when you have to mock an object and not an interface and Easymock can build proxy objects of them too. Not true. You can mock classes with jMock too: http://jmock.org/cglib.html :-) Ugo Thanks, I didn't find this. Anyway, I don't think we have to decide on one mock framework. Whoever writes the test decides, which one he prefers. And the difficult part is not to find the framework but to write test that really test the code in a relevant way which isn't always that easy. -- Reinhard
Re: Making tests suck less (was Re: ECM++)
On Mon, 25 Oct 2004, Ugo Cei wrote: Il giorno 25/ott/04, alle 09:54, Reinhard Poetz ha scritto: Anyway, I don't think we have to decide on one mock framework. Whoever writes the test decides, which one he prefers. Yes, that's a possibility. I am just thinking that it might be less confusing for would-be test writers if we settled on a recommended way. Yes, this would make it more clear as we also say we use Ant as our build tool and not the one that donnates a block can choose what he prefers, right. And the difficult part is not to find the framework but to write test that really test the code in a relevant way which isn't always that easy. Yup, indeed. That's for sure :-) -- Giacomo Pati Otego AG, Switzerland - http://www.otego.com Orixo, the XML business alliance - http://www.orixo.com
Making tests suck less (was Re: ECM++)
I wrote:
Currently, all the unit tests involving sitemap components fail because
the class org.apache.cocoon.components.ExtendedComponentSelector, which
is referenced by *.xtest files, has been removed.
Now, those tests depend on the deprecated ExcaliburTestcase class and it
would be nice to get rid of that dependency, but as a stopgap measure,
is there a class in ECM++ that can take its place just by substituting
the classname in its place?
Well, after thinking about the issue some more, I decided to rewrite all
the sitemap components' testcases following a different approach. I
intend to drop the SitemapComponentTestCase altogether and its
dependency on ExcaliburTestCase with it. First, what are the problems
with it?
1. Dependency on a deprecated class.
2. Writing tests is harder than it should be, with all those *.xtest files.
3. LAST BUT NOT THE LEAST: mostly we are not testing what we should be
testing. An example will clarify what I mean:
== ResourceExistsActionTestCase.java ==
public void testExistAction() throws Exception {
String src =
resource://org/apache/cocoon/acting/ResourceExistsActionTestCase.xtest;
Parameters parameters = new Parameters();
Map result = act(exist, src, parameters);
assertNotNull(Test if resource exists, result);
src =
resource://org/apache/cocoon/acting/ResourceExistsActionTestCase.abc;
...
}
=
This testcase is supposed to test the following method:
== ResourceExistsAction.java ==
public Map act(Redirector redirector, SourceResolver resolver, Map
objectModel, String src, Parameters parameters) throws Exception {
String resourceURI = parameters.getParameter(url, src);
Source source = null;
try {
source = resolver.resolveURI(resourceURI);
if (source.exists()) {
return EMPTY_MAP;
}
} catch (SourceNotFoundException e) {
// Do not log
} catch (Exception e) {
getLogger().warn(Exception resolving resource +
resourceURI, e);
} finally {
if (source != null) {
resolver.release(source);
}
}
return null;
}
But does it? What it does is test the SourceResolver.resolveURI method!
I would expect that the SourceResolver class was already bug-free and
had its own tests. There's no point testing that.
What should we be testing instead? Everything that can possibly go
wrong. What can go wrong here? Well, for instance, the programmer might
have forgotten to relase the source. We should be testing that the
source is always released. There's not much else here that can go wrong.
How do we test for that? One possibility is to use mock objects and set
expectations on them. We will give our action a mock SourceResolver
which returns a mock source. On those objects, we set an expectation
that the release method is indeed called, and verify that expectation.
Using the jMock library, the test looks like this (slightly edited to
simplify):
String src = don't care;
ResourceExistsAction action = new ResourceExistsAction();
Mock resolver = new Mock(SourceResolver.class);
Mock source = new Mock(Source.class);
resolver.expects(once()).method(resolveURI).with(same(src)).
will(returnValue(source.proxy()));
resolver.expects(once()).method(release).with(same(source.proxy()));
source.expects(atLeastOnce()).method(exists).will(returnValue(true));
Map result = action.act(null, (SourceResolver) resolver.proxy(),
objectModel, src, parameters);
assertSame(Test if resource exists, AbstractAction.EMPTY_MAP,
result);
resolver.verify();
source.verify();
If we forget to call the release method, the resolver.verify() call
will fail. You will notice also that the value of the src parameter is
totally irrelevant. We don't need the resolver to actually resolve the
URI. We just need it to return any implementation of the Source
interface and call release on that. One less test fixture to care of.
What we have gained is a less fragile, more relevant testcase.
In the coming days, I plan to rewrite all tests depending on
ExcaliburTestCase so that we can forget about it. Stay tuned.
Ugo
Re: Making tests suck less (was Re: ECM++)
Ugo Cei wrote: I wrote: Using the jMock library, the test looks like this (slightly edited to simplify): Have you considered using Easymock? IMHO its usage is more intuitive than jMock. -- Reinhard
Re: Making tests suck less (was Re: ECM++)
Ugo Cei wrote: I wrote: Using the jMock library, the test looks like this (slightly edited to simplify): Have you considered using Easymock? IMHO its usage is more intuitive than jMock. (see BlockDeployer test cases) -- Reinhard
Re: Making tests suck less (was Re: ECM++)
Il giorno 22/ott/04, alle 19:40, Reinhard Poetz ha scritto: Have you considered using Easymock? IMHO its usage is more intuitive than jMock. (see BlockDeployer test cases) After perusing the documentation and samples, I decided that I liked jMock more, but I didn't try EasyMock in practice. If you are familiar with it, you might try doing a version of the ResourceExistsActionTestCase that I just refactored, so we can do a comparison and decide which is better. However, I suspect it's more a matter of taste than anything else. Ugo -- Ugo Cei - http://beblogging.com/ smime.p7s Description: S/MIME cryptographic signature
