On Jun 5, 2010, at 4:03 PM, David Blevins wrote:
>
> On Jan 28, 2009, at 1:46 PM, David Blevins wrote:
>
>> I just did some work in the validation area and it occurs to me how much
>> work has yet to be done. It is definitely one of our most critical features
>> that users go "ooh" and "ahh" over.
>>
>> Code wise, its's one of those areas where the hardest part is being as
>> thorough as possible. I personally haven't been very thorough in my work
>> there and have just been adding validations as they occur to me and most
>> often without tests. Definitely shame on me.
>>
>> I just added a new class, ValidationAssertions, which will check the
>> ValidationFailedException failed exception thrown from the
>> ConfigurationFactory on depoy. It makes sure all the expected validation
>> issues were spotted, that there are i18n messages for each message key, that
>> there are the required three levels of messages, and that when formatted
>> there are no unfilled "{0}" sections. We get bit by that last one a lot.
>>
>> We really should add tests for all the checks/messages we have.
>>
>> There are also likely a ton of validations we don't do and should. We
>> should also get a list of all the annotations and make sure we have
>> validations for each of them and add the ones where we don't.
>>
>> Thoughts? Volunteers?
>
> If anyone is looking for some "get your feet wet" tasks, there are days and
> days worth of 2-3 hour tasks in this part of the system.
>
> Here's the main JIRA:
>
> https://issues.apache.org/jira/browse/OPENEJB-453
>
> Plenty of subtasks there and more that could be added. Of the existing
> checks we do, 90% of them are not tested. A user reported that one of them
> did not work just last week and fixing it and testing it was as simple as
> adding a test case like this:
>
> http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidCallbacksTest.java
>
> We really should have 100% coverage. If there is a message key for it in
> ../config/rules/Messages.properties, than there really should be a test for
> that scenario.
Hi Akila!
Basically, we need to go through this list and make sure there are tests to
challenge every message key:
http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
The stuff at the top is the oldest, so maybe better to start at the bottom :)
I suspect we may end up deleting a bunch of keys that are no longer used.
Might be good to do a quick grep on a key before bothering to write a test for
it. Something like the CheckInvalidCallbacksTest.java (see link above) is a
good example of a test. And definitely feel free to ask about a key if it
isn't clear how to create an invalid app that challenges it.
That's the overall goal. Feel free to tackle it anyway you like. I doubt
we'll complete the whole thing, but any improvement is excellent :) Certainly
this is one area which has an incredibly high value to users who are still
learning, yet it's also the least tested and is proving to be pretty buggy.
Maybe for the first run it's easier to pick one key and do just that one.
That'll give us a good opportunity to work out all the kinks. A first
contribution of any kind always involves a lot of questions -- compiling,
patching, JIRAs, etc, etc.
Anyway, welcome aboard! :)
-David
