Code Review Request: Generalise com.oracle.security.ucrypto tests

[Andrew Hughes]

----- Original Message -----
> Well, I have not looked at the complete webrev.
> But it looks to me that if the tests are generalized to cover more
> providers, then they should be moved to somewhere else instead of
> "com/oracle/security/ucrypto" directory which implies the regression
> tests for OracleUcrypto provider. Maybe somewhere under the
> /javax/crypto/ would be a better fit.

I agree and was thinking the same.  But I don't want to do it in this change
due to the noise it'll create.  I'll post a separate patch for this trivial
move.

> 
> One more concern that I have is that some functionalities tested in
> the
> tests may be *optional*. When the tests are developed for a specific
> provider such as OracleUcrypto provider, we know what it supports and
> doesn't support. However, when the tests are generalized to apply to
> all
> security providers, it gets a bit fragile. Even it works w/ current
> list
> of providers, once a new provider is added, the tests may very likely
> fail. The tests may have to be relaxed when it comes to these
> optional
> functionalities, e.g. digest cloning, key wrapping, etc.

So there is space to improve and specialise them.  I don't see this as a
problem.  In contrast, I think it's an advantage in that we can catch
where providers differ.  As I said in the first mail, the results show
that the tests pass for all the usual providers.  It's only if PKCS11
is turned on that issues are found, and finding these is very helpful
in us looking at turning this provider on by default.

A set of tests that test a provider that isn't even in OpenJDK seem
completely pointless to me.  To make them useful, I'd rather we improved
and generalised them rather than Oracle hiding them away in a private
repository.

> 
> Personally, I feel this is a bit more than "regression tests"...
> Valerie
> 
> On 03/29/12 07:43, Andrew Hughes wrote:
> > Hi all,
> >
> > As discussed in:
> >
> > http://mail.openjdk.java.net/pipermail/security-dev/2012-March/004615.html
> >
> > this webrev:
> >
> > http://cr.openjdk.java.net/~andrew/enc_tests/webrev.01/
> >
> > updates the tests as follows:
> >
> > 1.  Tests all available providers, not just ucrypto which is
> > unavailable on OpenJDK.
> > 2.  No longer fails the whole test due to a single failure, but
> > provides a cumulative result instead.
> > 3.  All output goes to System.err so that stack traces correspond
> > to other output for debugging.
> >
> > The tests pass on all providers, with the exception of PKCS11 which
> > has issues on AES that we're still looking onto.
> > There were issues with Digest too, but this should be resolved by
> > the fix described in
> > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6414899
> >
> > changeset:   4908:cdc128128044
> > parent:      4888:0194fe5ca404
> > user:        valeriep
> > date:        Thu Jan 05 18:18:04 2012 -0800
> > summary:     6414899: P11Digest  should support cloning
> >
> > Does this look ok?  If so, can I have a bug ID to commit it and
> > which tree should it go to?
> >
> > Thanks,
> 
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07