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

Reply via email to