You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Ugo Cei <u....@cbim.it> on 2003/10/09 01:34:19 UTC
ResolverImpl test case broken again?
I'm a bit confused. First of all, I don't give a damn about
ResolverImpl. It's deprecated and some time ago David Crossley proposed
to remove the ResolverImplTestCase, because it caused the build to fail
if deprecated classes were not compiled.
But now Carsten has fixed the problem
(http://nagoya.apache.org/bugzilla/show_bug.cgi?id=20096) and the build
should run fine even without deprecated classes (but does it? See below).
The problem is that the test itself fails:
Testcase: testCatalogPropertiesAvailable took 0.009 sec
Caused an ERROR
null
java.lang.ClassCastException
at
org.apache.cocoon.components.resolver.test.ResolverImplTestCase.setUp(ResolverImplTestCase.java:282)
I find it strange that nobody else noticed the problem, so maybe it has
to do with some problem local to my machine (or maybe nobody ever runs
the test suite). So I run "build test" on the 2.1.2 release and lo and
behold ... it fails as well!
Did we really release with a broken test suite? I find it hard to
believe and I'm asking confirmation from someone else before I file an
issue in bugzilla.
From what I can understand, the bug is due to a mismatch between the
source:
282: resolverImpl = (DefaultEntityResolver) manager.lookup(role);
and the ResolverImplTestCase.xtest file:
<role name="org.apache.excalibur.xml.EntityResolver"
shorthand="entity-resolver"
default-class="org.apache.cocoon.components.resolver.ResolverImpl">
</role>
Well, of course ResolverImpl implements EntityResolver but does not
extend DefaultEntityResolver, thus the ClassCastException.
Let's see what if I can do a quick fix (actually I'm writing this mail
non post-facto, but as a kind of loud reasoning while I'm working on the
problem) ... a few seconds later:
./build.sh test
...
BUILD SUCCESSFUL
Total time: 1 minute 54 seconds
OK, cool. Let's see if it runs without deprecated classes:
./build.sh test
...
java.lang.ClassNotFoundException:
org.apache.cocoon.components.resolver.ResolverImpl
...
BUILD FAILED
Oh, shit! Let's try this on the 2.1.2 release ... same error! I'm
baffled. Did bug 20096 rear its ugly head again? Was closing it a
side-effect of Belgian beers? ;-).
So, to recapitulate:
- Bug #20096 is to be reopened.
- The test suite is broken, but we have a fix for that (patch is
attached below).
[Of course it's entirely possible that I'm missing something obvious,
being 1:23PM and maybe I'm still under the influence of the above
mentioned beers.]
In the end, do we keep ResolverImplTestCase and try to fix bug #20096
again, or do we drop it and go on? In my case, going on means writing
some tests for Woody before I try to implement a feature I have in mind.
I was just preparing to do this when I stumbled upon
ResolverImplTestCase, so I'm inclined to say "kill the bastard and just
go on".
What do you think?
Ugo
P.S.: patch follows:
Index: ResolverImplTestCase.java
===================================================================
RCS file:
/home/cvs/cocoon-2.1/src/test/org/apache/cocoon/components/resolver/test/ResolverImplTestCase.java,v
retrieving revision 1.7
diff -u -r1.7 ResolverImplTestCase.java
--- ResolverImplTestCase.java 30 Sep 2003 13:36:33 -0000 1.7
+++ ResolverImplTestCase.java 8 Oct 2003 23:33:03 -0000
@@ -59,10 +59,10 @@
import org.apache.avalon.excalibur.testcase.ExcaliburTestCase;
import org.apache.avalon.framework.activity.Disposable;
import org.apache.avalon.framework.activity.Initializable;
+import org.apache.avalon.framework.component.Component;
import org.apache.avalon.framework.context.DefaultContext;
import org.apache.cocoon.Constants;
import org.apache.excalibur.xml.EntityResolver;
-import org.apache.excalibur.xml.DefaultEntityResolver;
import org.apache.cocoon.environment.commandline.CommandLineContext;
import org.apache.cocoon.util.IOUtils;
import org.xml.sax.InputSource;
@@ -231,7 +231,7 @@
"<!ENTITY shy \"­\" ><!--=soft hyphen-->\n" +
"";
private DefaultContext context;
- private DefaultEntityResolver resolverImpl;
+ private EntityResolver resolverImpl;
private File workDir;
private File commandlineContextDir;
@@ -279,7 +279,7 @@
super.setUp();
String role = EntityResolver.ROLE;
- resolverImpl = (DefaultEntityResolver) manager.lookup(role);
+ resolverImpl = (EntityResolver) manager.lookup(role);
assertNotNull("ResolverImpl is null", resolverImpl);
}
@@ -346,7 +346,7 @@
super.tearDown();
if (resolverImpl != null) {
- manager.release(resolverImpl);
+ manager.release((Component) resolverImpl);
resolverImpl = null;
}
}
Re: ResolverImpl test case broken again?
Posted by David Crossley <cr...@indexgeo.com.au>.
David Crossley wrote:
<snip/>
>
> If no-one says anything in the next few days, then i will remove the
> resolver testcases. They served their purpose here in Cocoon while
> developing the entity-resolver stuff, to ensure that it worked on
> all platforms.
Okay, the deprecated entity resolver testcases are now removed
from 2.1 and 2.2
--David
Re: ResolverImpl test case broken again?
Posted by David Crossley <cr...@indexgeo.com.au>.
Ugo Cei wrote:
> David Crossley wrote:
> > I tested your patch and that works fine on my system.
> > Well done, i believe that you have fixed it. Would you please commit.
>
> I'll do this ASAP.
>
> > So we can leave the resolver test cases there until we find something
> > better to do with them. We really need to clean up all of the testcases.
>
> Having slept over the issue (not much, just a little more than 4 hours
> :-(), these are my thoughts:
>
> We should strive for 100% test coverage. Removing a test case without
> removing the class being tested is no good, in my book.
>
> Should we remove the ResolverImpl class? As far as I can see, it's not
> used anywhere in our codebase, apart from the test case. On the other
> hand, it's not @deprecated, even though it implements a deprecated
> interface. Should we call a vote on the issue?
If no-one says anything in the next few days, then i will remove the
resolver testcases. They served their purpose here in Cocoon while
developing the entity-resolver stuff, to ensure that it worked on
all platforms.
--David
Re: ResolverImpl test case broken again?
Posted by Ugo Cei <u....@cbim.it>.
David Crossley wrote:
> I tested your patch and that works fine on my system.
> Well done, i believe that you have fixed it. Would you please commit.
I'll do this ASAP.
> So we can leave the resolver test cases there until we find something
> better to do with them. We really need to clean up all of the testcases.
Having slept over the issue (not much, just a little more than 4 hours
:-(), these are my thoughts:
We should strive for 100% test coverage. Removing a test case without
removing the class being tested is no good, in my book.
Should we remove the ResolverImpl class? As far as I can see, it's not
used anywhere in our codebase, apart from the test case. On the other
hand, it's not @deprecated, even though it implements a deprecated
interface. Should we call a vote on the issue?
> For example, do we need the XMLform stuff?
There are people out there (me included) who have applications based on
this stuff and migration to Woody is not a simple process. I'd say keep it.
> --David
Ugo
--
Ugo Cei - Consorzio di Bioingegneria e Informatica Medica
P.le Volontari del Sangue, 2 - 27100 Pavia - Italy
Phone: +39.0382.525100 - E-mail: u.cei@cbim.it
Re: ResolverImpl test case broken again?
Posted by David Crossley <cr...@indexgeo.com.au>.
Ugo Cei wrote:
<snip/>
> Did we really release with a broken test suite? I find it hard to
> believe and I'm asking confirmation from someone else before I file an
> issue in bugzilla.
Erk. Yes it seems that we did do the release without running
'build test'. I just added a note to the Wiki CocoonReleaseHowTo
to remind us.
<snip/>
> - Bug #20096 is to be reopened.
> - The test suite is broken, but we have a fix for that (patch is
> attached below).
I tested your patch and that works fine on my system.
Well done, i believe that you have fixed it. Would you please commit.
So we can leave the resolver test cases there until we find something
better to do with them. We really need to clean up all of the testcases.
For example, do we need the XMLform stuff?
--David