openejb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Karan Malhi <karan.ma...@gmail.com>
Subject Re: Flushing out validation
Date Tue, 27 Jul 2010 04:33:56 GMT
The testing of validation keys is almost complete. We have 95% test coverage
for all validation keys .
Here is the report:
https://cwiki.apache.org/confluence/display/OPENEJB/Validation+Keys+Audit+Report

The untested keys are left due to certain reasons listed below:-

A. Here are the keys where I was not sure how to test them:

   1. cannot.validate
   2. client.missingMainClass
   3. missing.dependent.class
   4. misslocated.class


B. List of keys which cannot be tested because validation for these is
bypassed if we do not provide a real jar file. While testing, we
programmaticaly create an EjbJar instead of a physical jar file
1. abstractAnnotatedAsBean
2. interfaceAnnotatedAsBean
3. multiplyAnnotatedAsBean

C. Some of the keys were commented out from the messages.properties file
because they were unused in our validation code or validation of these is
based on the draft schema and the feature is yet to be implemented properly
for the final  ejb3.1 schema. If somebody feels i have wrongly commented out
a key, please feel free to let me know and I can fix it. Also some keys were
duplicates, not in the literal sense, but in that they were being used in
testing the same validation logic. Commented those out too.

On Fri, Jul 16, 2010 at 12:53 PM, Karan Malhi <karan.malhi@gmail.com> wrote:

>
> I have been puzzled that some of the keys which were being tested were no
> where to be found in the source code. I used eclipse search, wrote a little
> asm utility, used find and grep, to no avail.
>
> Here is an example where i was using find to search for they key
> xml.businessLocal.ejbObject
>
>  find  -name *.java -exec grep "xml.businessLocal.ejbObject" '{}' \; -print
>
> I got 1 result and that too was in a test case testing for this validation
> key. This puzzled me because I thought, how can a key be tested, if it was
> not even being used in our Validation code? So I started relaxing my search
> criteria a bit i.e. just searched for the substring of the key, in this
> case, here was the find command
>
> find  -name *.java -exec grep "businessLocal" '{}' \; -print
>
> The above gave me multiple results, as you can guess, most of them were
> irrelevant. However, i found one little code snippet in the result, which
> solved the mystery for me. Here it is
> fail(b, "xml." + tag + ".businessLocal", clazz.getName());
>
> The key was being built by appending strings , there was no way the key
> could have shown up in the original search.
>
> In an earlier email I had mentioned that each key has a comment, which is
> nothing but the code which uses it. Had that been the case for all the keys,
> it would have taken me minutes to search for that code and figure out how
> the key is being used and that would have helped figure out why my tests
> were failing.
>
> Anyways, I would like to propose a new addition to the Message.properties
> file.
> First, lets look at an example key entry in this file
> # warn(b, "unused.ejb.create", b.getEjbClass(), ejbCreate.getName(),
> paramString, create.toString());
> 1.unused.ejb.create       Unused ejbCreate method
> 2.unused.ejb.create       Unused ejbCreate method: {1}({2})
> 3.unused.ejb.create       Create method will never be called.  The bean
> class {0} defines the create method {1}({2}), but there is no matching
> {3}({2}) method in the home or local-home interfaces.
>
> As you can see, the comment specifies the code where this key is being
> used. Instead of this info being a comment, I would like to modify it as
>
> 0.unused.ejb.create       warn(b, "unused.ejb.create", b.getEjbClass(),
> ejbCreate.getName(), paramString, create.toString());
> 1.unused.ejb.create       Unused ejbCreate method
> .....
> ......
>
> So instead of it being a comment, now i can load that info and write a
> utility which will verify that the source code information for each key
> exists and that the information is correct. Sometimes changes to source code
> may be made, but this file might not be updated, in those scenarios, the
> utility will be very helpful in keeping things in sync.
>
>
> Thoughts?
>
>
>
> On Fri, Jul 16, 2010 at 12:29 AM, David Blevins <david.blevins@visi.com>wrote:
>
>>
>> On Jul 15, 2010, at 7:42 PM, Karan Malhi wrote:
>>
>> > The key persistenceContextRef.noMatches is also not used. Should i just
>> > start deleting these keys from Messages.properties, or probably comment
>> them
>> > out?
>>
>> If it is something worth checking and not covered, we should just add
>> checks for it if possible.  Fine to comment it out with a TODO in the
>> meantime.
>>
>> -David
>>
>> >
>> > On Thu, Jul 15, 2010 at 9:23 PM, David Blevins <david.blevins@visi.com
>> >wrote:
>> >
>> >>
>> >> On Jul 15, 2010, at 5:26 PM, David Jencks wrote:
>> >>
>> >>> I think that particular one's xml element/jee tree class got renamed
>> to
>> >> ConcurrencyManagementType late in the spec.  I don't know if it's being
>> >> handled in code yet...so this may be of no use :-\
>> >>
>> >> Renamed and relocated from assembly descriptor to under <session>
>> >>
>> >> Anyway, here's the code that drives concurrency-attribute:
>> >>
>> >>   /*
>> >>    * @Lock
>> >>    */
>> >>   if (sessionBean.getConcurrencyManagementType() ==
>> >> ConcurrencyManagementType.CONTAINER) {
>> >>       processAttributes(new
>> >> ConcurrencyAttributeHandler(assemblyDescriptor, ejbName), clazz,
>> >> inheritedClassFinder);
>> >>   } else {
>> >>       checkAttributes(new
>> ConcurrencyAttributeHandler(assemblyDescriptor,
>> >> ejbName), ejbName, ejbModule, classFinder,
>> "invalidConcurrencyAttribute");
>> >>   }
>> >>
>> >> It uses the same abstraction for method based processing as transaction
>> >> attribute does (and also revealing of why it's hard to change that code
>> to
>> >> the different element)
>> >>
>> >>   /*
>> >>    * TransactionAttribute
>> >>    */
>> >>   if (bean.getTransactionType() == TransactionType.CONTAINER) {
>> >>       processAttributes(new
>> >> TransactionAttributeHandler(assemblyDescriptor, ejbName), clazz,
>> >> inheritedClassFinder);
>> >>   } else {
>> >>       checkAttributes(new
>> TransactionAttributeHandler(assemblyDescriptor,
>> >> ejbName), ejbName, ejbModule, classFinder,
>> "invalidTransactionAttribute");
>> >>   }
>> >>
>> >>
>> >> On the assembler side, the code for the above and security attributes
>> all
>> >> uses the same algorithm (MethodAttributeInfo +
>> >> MethodInfoUtil.resolveAttributes).
>> >>
>> >> Anyway, skip that message key because the relating code will greatly
>> change
>> >> at some painful point :)
>> >>
>> >>
>> >> -David
>> >>
>> >>
>> >>>
>> >>> On Jul 15, 2010, at 4:40 PM, Karan Malhi wrote:
>> >>>
>> >>>> What needs to be done with keys which are listed in
>> Messages.properties
>> >> but
>> >>>> are not used anywhere in the java code.? For example,
>> >>>> xml.invalidConcurrencyAttribute cannot be found in any .java file
in
>> the
>> >>>> ejb-core project, so I am assuming that there is not validation
for
>> >> this.
>> >>>>
>> >>>> On Thu, Jul 15, 2010 at 5:57 PM, Karan Malhi <karan.malhi@gmail.com>
>> >> wrote:
>> >>>>
>> >>>>> In the messages.properties, some of the keys have nice comments
>> above
>> >> them,
>> >>>>> like fail() or warn(). This gives a good idea to the test author
>> >> whether to
>> >>>>> test for a warning or failure. Please try and make sure if you
add
>> keys
>> >> to
>> >>>>> this file, also supplement the keys with the little comment.
>> >>>>>
>> >>>>>
>> >>>>> On Wed, Jul 14, 2010 at 11:38 PM, David Blevins <
>> >> david.blevins@visi.com>wrote:
>> >>>>>
>> >>>>>>
>> >>>>>> On Jun 10, 2010, at 10:48 PM, David Blevins wrote:
>> >>>>>>
>> >>>>>>>> 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.
>> >>>>>>>
>> >>>>>>> Basically, we need to go through this list and make
sure there are
>> >> tests
>> >>>>>> to challenge every message key:
>> >>>>>>
>> >>>>>> FYI, to those who haven't seen it.  This is pretty darn
cool!!
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>
>> https://cwiki.apache.org/confluence/display/OPENEJB/Validation+Keys+Audit+Report
>> >>>>>>
>> >>>>>> Innovative stuff!
>> >>>>>>
>> >>>>>> -David
>> >>>>>>
>> >>>>>>
>> >>>>>
>> >>>>>
>> >>>>> --
>> >>>>> Karan Singh Malhi
>> >>>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> --
>> >>>> Karan Singh Malhi
>> >>>
>> >>>
>> >>
>> >>
>> >
>> >
>> > --
>> > Karan Singh Malhi
>>
>>
>
>
> --
> Karan Singh Malhi
>



-- 
Karan Singh Malhi

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message