RFR[10] 8186057: TLS interoperability testing between different Java versions

sha.jiang at oracle.com sha.jiang at oracle.com
Tue Nov 28 02:35:27 UTC 2017


Hi Artem,
Please review the new webrev at: 
http://cr.openjdk.java.net/~jjiang/8186057/webrev.07/
Please see my comments below.

>>>>>
>>>>> One question about Compatibility.caseStatus(). What's the case 
>>>>> when you expect a timeout and no client output? Should it always 
>>>>> result to a test case failure?
>>>> I'm not sure understanding your question. Did you mean server or 
>>>> client timeout?
>>>> I think client should output something.
>>> My questing is - should timeout be always considered as a failure? 
>>> Why don't you just check if server or client result is FAIL?
>>>
>>> if (clientStatus == FAIL || serverStatus == FAIL) {
>>>     return FAIL;
>>> }
>>>
>>> return PASS;
>> Some cases are negative cases, their status should be EXPECTED_FAIL.
> That's fine, I got that.
>> A server or client timeout would lead to the case to fail, if the 
>> failure is not expected.
>> If a server starts normally, but client doesn't start due to a 
>> negative case, the server should get timeout. But this failure is 
>> expected.
>> If a server starts normally, and client get a unexpected timeout, 
>> this failure would make the case to fail.
> This logic looks unnecessary complex to me. The more complex it is, 
> the more likely we get a bug there. I think TIMEOUT status is not 
> necessary here. If a failure is expected on one side, the other side 
> should fail as well (no matter if it's a timeout or an exception).
>
> If I were you, I would simplify this method like this
>
> if (client == EXPECTED_FAIL && server == EXPECTED_FAIL) {
>     return EXPECTED_FAIL;
> }
>
> if (client == PASS && server == PASS) {
>     return PASS;
> }
>
> return FAIL;
>
> If I am not missing something, the pseudo-code above looks better and 
> simpler.
1. This test designs 5 status:
SUCCESS, UNEXPECTED_SUCCESS, FAIL, EXPECTED_FAIL and TIMEOUT
The main purpose is making the test report more clear.
A case fails that may be due to:
Unexpected failure;
No failure, but it is expected to get some failure;
Timeout, although it is also not expected, this failure may have nothing 
to do with SSL/TLS exceptions, like SSLHandshakeException.
The test just wants to distinguish these different situations. I assume 
users generally just look through the report table, but not the detail logs.

2. This test really treats TIMEOUT as failure. Server and/or client 
timout, it should result in the case gets FAIL or EXPECTED_FAIL.

3. Your above code snippet doesn't cover all possible result status.
For example, client == PASS && server == PASS, but this case may be 
expected to get some failure. The status should be UNEXPECTED_SUCCESS in 
my design.
Again, some more status are used to make the report table could give 
more information before investigating details.

4. The updated webrev havs merged two if-clauses to simplify the method 
Compatibility.caseStatus() a bit.

Best regards,
John Jiang



More information about the security-dev mailing list