[Pki-devel] Add self tests to tomcat TPS

Ade Lee alee at redhat.com
Tue Aug 20 15:27:34 UTC 2013


On Mon, 2013-08-19 at 13:35 -0500, Endi Sukma Dewata wrote:
> On 8/16/2013 3:24 PM, Ade Lee wrote:
> > Please review.
> > Thanks,
> > Ade
> 
> The tests work, some comments:
> 
> 1. There's a typo in UserMessages.properties:
> 
>    CMS_SELFTESTS_TPS_VALIDITY_DESCRIPTION=... TKS is valid.
> 
Fixed.

> 2. When throwing a new exception, it would be better to attach the 
> original exception so it can be traced back to the actual line 
> triggering the error. The SelfTest exception constructors don't seem to 
> take any exception parameter though, so this will require some changes 
> in the exception classes. This can be done separately later.
> 
Yes, lets do this separately.

> 3. Just minor thing, but there seems to be a little bit too many code 
> blocks & indentations. For clarity, the main code execution shouldn't be 
> in the if-then-clause unless necessary. For example, instead of this:
> 
>    if (something != null) {
>        do something
>    } else {
>        throw new Exception();
>    }
> 
> it can be simplified into this:
> 
>    if (something == null) {
>        throw new Exception();
>    }
> 
>    do something
> 
> So the if-then-clause is only used to divert from the main execution if 
> there's an error.
> 
Agreed.  I was taking the old self tests as a template and forgot to
change that bit.

Also added a check for null-ness in the function toLowerCaseSubsystem()
as found by Andrew.

Pushed to master.





More information about the Pki-devel mailing list