mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Jakl <jakl.mich...@gmail.com>
Subject Re: Some comments about the last VYSPER commits
Date Sat, 04 Jul 2009 09:11:01 GMT
Hi!

Thanks for the feedback, good to know that somebody does a sanity
check from time to time ;)

On Sat, Jul 4, 2009 at 01:01, Emmanuel Lecharny<elecharny@apache.org> wrote:
> tests can be defined using annotations, like :
>
>   @Test
>   public void testPublishNoSuchNode() throws Exception {
>
> if Junit 4.4+ is used.
> Junit 4.4(+ has the big advantage to allow the use of @beforeClass and
> @AfterClass annotations, allowing you to run a method before and after *all*
> the tests. A very powerful feature.

Currently we're only using JUnit 3, which doesn't support any of these
annotations.

Concerning before-/afterClass methods: I think that a test should be
as independent as possible. As long as we're not experiencing serious
setUp and tearDown times I'd rather have each test setup its own
environment.

Our whole test-suite runs in 1.5 seconds on my computer. Most of the
time I'm executing only the tests for pubsub or even the test for the
feature I'm implementing, which takes even less time.

Of course these annotations are nice and methods could be given more
intuitive names, ATM this would cause more changes than just switching
the junit library.

> http://svn.apache.org/viewvc/mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/PubsubFeatures.java?rev=791030&view=auto
>
> ...
> public class PubsubFeatures {
>   public static final PubsubFeature access_authorize = new
> PubsubFeature("access-authorize", "The default access model is
> \"authorize\".", "OPTIONAL", "Nodes Access Models");
> ... (and all the following)
>
> constants should be uppercased, accordingly to the current coding convention
> we are using. These lines should be :
>
> ...
> public class PubsubFeatures {
>   public static final PubsubFeature ACCESS_AUTHORIZE = new
> PubsubFeature("access-authorize", "The default access model is
> \"authorize\".", "OPTIONAL", "Nodes Access Models");
>
> That helps to keep the code consistant :
>
> ...
>        infoElements.add(new Feature(NamespaceURIs.XEP0060_PUBSUB));
>        infoElements.add(new Feature(PubsubFeatures.access_open.toString()));
> ...
>
> Would be better as :
>
> ...
>        infoElements.add(new Feature(NamespaceURIs.XEP0060_PUBSUB));
>        infoElements.add(new Feature(PubsubFeatures.ACCESS_OPEN.toString()));
> ...

Thanks, done. The file was generated semi-automatically and I was too
lazy to go over the names and change them to uppercase yesterday.

> That's pretty much it, the remaining code is just fine !

Thanks!


Michael

Mime
View raw message