RFR(XXS): 8141146: Add Error hierarchy to test library

[Alexandre (Shura) Iline]

Hi,

Kirill, I have two comments:
1. I think that the enhancement description is not thorough enough. You told me more in person than it is in the description.
2. I do not see how the two exception types suggested in the diff are any useful without all the supporting code.

That all said, here is what I believe is a motivation for the enhancement …

When, while writing an application code, one uses asserts in the code  to verify some assumptions about the state of the application data, etc. (I)

While writing tests, such as some unit or API tests with TestNG, one often uses the asserts to verify that the tested code behaves as expected. (II) 

I have no intention of giving strict definitions to the terms, but the purpose of using asserts is, clearly, different in I and II.

But then a test code could grow bigger, acquire libraries, and become needing assertion usage of the first type to verify state of the test system itself.

What the enhancements suggests therefore, is a way to easily distinguish the two types of usage of assertions: 
 * assertions for state of the test system
 * assertions that the tested code is doing what’s it’s supposed to
the distinctions would clearly be made both in the test source code and the logs, should a test like this fail.

There is also third kind of asserts usage which we happen to deal with often: verifying assumptions about the environment. This, I thought, would be a part of the enhancement.

I do not know if the above helps: this is how I heard guys sharing the idea with me.

I do not have any data to back this up. To get the data, one would have to do a thorough analysis of the assertions usage across a test set of a representative size. I am pretty sure that the asserts for the test system used way less often … but the test libraries keep growing.

Shura

> On Nov 3, 2015, at 3:43 AM, Kirill Zhaldybin <Kirill.Zhaldybin at oracle.com> wrote:
> 
> Hi Christian,
> 
> On 02.11.2015 22:09, Christian Tornqvist wrote:
>> Hi Kirill,
>> 
>>> I think it indicates that we need a separate error for test bug/environment issue. It will allow us to identify test/env issues easily and assign them directly to SQE avoiding spending any dev time.
>> 
>> I'm still very sceptic how you'd reliably separate the issues into test/environment/product issues and what "SQE avoiding spending any dev time" means?
> 
> I agree that not all issues could be reliably separated but for these type of issues no changes will be introduced. As before they will be considered as a product issue.
> 
> Nonetheless some issues, for example, missed methods in compiler/intrinsics/mathexact/sanity/MathIntrinsic.java are likely test bugs.
> The another example is wrong generated classfile in compiler/c1/6932496/Test6932496.java.
> gc/g1/humongousObjects/TestHumongousThreshold.java has assertion that checks internal test logic and the test would only benefit from separate test bug assert.
> 
> Environment issue could used in case of different I/O issues that are not connected with test logic, already occupied TCP/IP ports etc.
> 
> Shura could have more real life example for these cases.
> 
> Thank you.
> 
> Regards, Kirill
> 
>> 
>> Again, I'd love to see some examples of why this would be a worthwhile addition.
>> 
>> Thanks,
>> Christian
>> 
>> -----Original Message-----
>> From: Kirill Zhaldybin [mailto:kirill.zhaldybin at oracle.com]
>> Sent: Monday, November 2, 2015 1:57 PM
>> To: Christian Tornqvist <christian.tornqvist at oracle.com>; 'hotspot-dev' <hotspot-dev at openjdk.java.net>
>> Subject: Re: RFR(XXS): 8141146: Add Error hierarchy to test library
>> 
>> Hi Christian,
>> 
>> We already have more than 2k statements like "throw new testbug" in hotspot/test.
>> I think it indicates that we need a separate error for test bug/environment issue. It will allow us to identify test/env issues easily and assign them directly to SQE avoiding spending any dev time.
>> 
>> I created a sub-task on new Asserts framework here -
>> https://bugs.openjdk.java.net/browse/JDK-8141204
>> We are really appreciate your comments.
>> 
>> Thank you.
>> 
>> Regards, Kirill
>> 
>> 
>> On 02.11.2015 21:20, Christian Tornqvist wrote:
>>> Hi Kirill,
>>> 
>>> I'm still doubting how useful this would be, can you give some examples of where these different exceptions would be used and what the benefit would be? How would this be used with the current Assert class?
>>> 
>>> Thanks,
>>> Christian
>>> 
>>> -----Original Message-----
>>> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On
>>> Behalf Of Kirill Zhaldybin
>>> Sent: Monday, November 2, 2015 12:56 PM
>>> To: hotspot-dev <hotspot-dev at openjdk.java.net>
>>> Subject: RFR(XXS): 8141146: Add Error hierarchy to test library
>>> 
>>> Hi,
>>> 
>>> Could somebody please review this change?
>>> It adds EnvironmentIssue and TestBug classes for environment issue and test bug correspondingly to the test library.
>>> It is the first part of JDK-8065651: Enhance Asserts to throw different exceptions for different types of assertions.
>>> 
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8141146
>>> WebRev:
>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8141146/webrev.00/
>>> 
>>> Thank you.
>>> 
>>> Regards, Kirill
>>> 
>> 
>> 
>