[9] RFR: 8074784: Additional tests for XML DSig API

Sean Mullan sean.mullan at oracle.com
Thu Jul 16 16:58:11 UTC 2015


Hi Artem,

The updated test looks good.

Thanks,
Sean

On 06/26/2015 01:57 PM, Artem Smotrakov wrote:
> Hi Sean,
>
> I added new test cases to GenerationTests, please take a look:
>
> http://cr.openjdk.java.net/~asmotrak/8074784/webrev.01/
>
> Artem
>
> On 06/16/2015 07:23 PM, Sean Mullan wrote:
>> On 05/20/2015 04:16 PM, Artem Smotrakov wrote:
>>> Hi Sean,
>>>
>>> Yes, at first, I thought about updating the existing tests in
>>> test/javax/xml/crypto/dsig directory. But then I noticed that both
>>> GenerationTests and ValidationTests has ~30 test cases. And new
>>> Detached.java test contains >30 test cases. If one of test cases fails,
>>> JTREG will show that full test failed. As a result, it may hide failures
>>> of other test cases (an engineer should look at logs carefully).
>>
>> You could always change that to continue on error and run all tests
>> before reporting a failure. That was just originally the way the test
>> was created, it isn't set in stone.
>>
>>> Also it
>>> may be better to split tests if possible when some tools for automated
>>> failures analysis is used (for example, Java SQE uses such a tool).
>>>
>>> That was the main reason why I added a separate test. Not sure if
>>> performance may be an issue, I have not done any measurement.
>>
>> Yes, but there is still a lot of common infrastructure in this test
>> and ValidationTests that could be combined. You have made use of
>> lambdas and streams to create a nice way to structure and run the
>> different variants of tests. But a detached signature is really just
>> another variant, and I think your test could be easily adapted to also
>> test enveloped and enveloping signatures, as well as the other cases
>> in ValidationTests. So, it would be nice to combine these tests. I
>> would be ok with filing a follow-on issue to do that, though.
>>
>> I noticed a few typos:
>>
>> 151: s/fot/for
>> 332: s/transfrom/transform/
>> 418: s/validaition/validation/
>>
>> I also would recommend reordering the imports in alphabetical order.
>>
>> Looks fine otherwise.
>>
>> Thanks,
>> Sean
>>
>>>
>>> Artem
>>>
>>> On 05/20/2015 10:52 PM, Sean Mullan wrote:
>>>> Hi Artem,
>>>>
>>>> Is there a reason this needs to be a separate test? It seems like it
>>>> would be better to fold it into the existing GenerationTests and
>>>> ValidationTests in the test/javax/xml/crypto/dsig directory, so you
>>>> could reuse common code.
>>>>
>>>> Thanks,
>>>> Sean
>>>>
>>>> On 05/12/2015 11:32 AM, Artem Smotrakov wrote:
>>>>> Hello,
>>>>>
>>>>> Please review a new test for generating and validation of detached XML
>>>>> digital signatures.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8074784
>>>>> Webrev: http://cr.openjdk.java.net/~asmotrak/8074784/webrev.00/
>>>>>
>>>>> Artem
>>>
>



More information about the security-dev mailing list