RFR JDK-8080641: JEP-JDK-8042880 : Implement new tests on Project Coin

[Alex Buckley]

Thanks for the pointers.

I remember now: array components can't be used as resources because, as 
variables, they are never final or effectively final. (Only an array 
variable itself may be final or effectively final, not its components.) 
That's why the grammar only allows ExpressionName and FieldAccess, not 
ArrayAccess.

But, since array components are variables, I would have expected them to 
be tested in TwrVarKinds rather than TwrForVariable2. And, I would have 
expected the negative tests in TwrForVariable2 for a conditional 
expression and a null-with-cast to be located in TwrForVariable3 where 
other (illegal) expressions are tried. Tests should be written in the 
right place if we want people to read them.

WeirdTwr_WithVar is misnamed and unnecessary. The single t-w-r statement 
that it contains could live perfectly well in TwrForVariable1, near the 
nice new cases for assertCloseCount(7) and  assertCloseCount(10).

Alex

On 10/19/2015 5:35 PM, Sergei Pikalev wrote:
>
> Hi Alex,
>
> These test are additional to existing test, and developers recommended me
> not to create new files but enrich existing as far as possible.
>
> Array components assertions are located at TwrForVariable2.
>
> Other discussed issues are in TwrForVariable1. Null cases 'prove' that
> null does
> not generate exceptions beyond the t-w-r block. Positive part checks
> initialized
> resources.
>
> Some of this assertions came from developers' unit test. Some was added
> by me.
>
> Sergi
>
> On 20.10.2015 02:57, Alex Buckley wrote:
>> I don't see any testing of array components as resources, or testing
>> that a resource declared outside the t-w-r statement can either be
>> null or is guaranteed to initialize (as discussed in a separate mail).
>>
>> Alex
>>
>> On 10/19/2015 8:05 AM, Sergei Pikalev wrote:
>>>
>>> Hi All,
>>>
>>> http://cr.openjdk.java.net/~shurailine/8080641/webrev.01/
>>>
>>> There are updated tests with respect to Alex's remarks.
>>>
>>> Thanks,
>>>      Sergi
>>>
>>> On 16.10.2015 02:20, Alex Buckley wrote:
>>>> Hi Sergei,
>>>>
>>>> Most of the new regression tests are about conformance to the spec,
>>>> rather than probing some javac-internal treatment of the t-w-r
>>>> statement. I hope someone from JCK-Compiler can comment on their
>>>> suppor for the expanded t-w-r statement.
>>>>
>>>> That said, here are some comments on the new tests:
>>>>
>>>> - ResourceVariableOutsideTry - @summary should be "Resource variable
>>>> should be in scope throughout try, catch, and finally blocks".
>>>> Accessibility is not relevant to a local variable like c.
>>>>
>>>> - TwrAndAnonymousClass - this is a positive test when an anonymous
>>>> class implements AutoCloseable, but what about a negative test when an
>>>> anonymous class doesn't implement AutoCloseable?
>>>>
>>>> - TwrAndGenerics - should be called TwrAndTypeVariables - plus same
>>>> comment as above (type variables S and T _not_ extending
>>>> AutoCloseable).
>>>>
>>>> - TwrAndLambda - same comment as above (lambda expressions and method
>>>> references _not_ implementing AutoCloseable).
>>>>
>>>> - TwrVarKinds - good test, but if you're doing to test four kinds of
>>>> variables then you may as well test all seven (JLS 4.12.3). In
>>>> particular, array components should be tested. You should test using
>>>> components from an array of AutoCloseables, and components of
>>>> components from an array of an array of AutoCloseables. (The second
>>>> situation in particular will expose any mistreatment of array
>>>> components by javac -- if javac accidentally erases the array-ness of
>>>> a resource variable, then in the first situation it will end up with
>>>> an AutoCloseable and that's ordinary enough that everything might
>>>> still work, but in the second situation it ends up with an array of
>>>> AutoCloseables and that might blow up some logic.)
>>>>
>>>> - WeirdTwr_WithVar - there is nothing weird or strange here. It's just
>>>> one resource variable aliasing another, which was possible already.
>>>> You're right to extend it so that a resource variable declared by
>>>> 'try' aliases a resource variable declared outside 'try' -- also do it
>>>> the other way around. Also, need a test where the resource variable
>>>> declared outside 'try' is null.
>>>>
>>>> - What's truly new when a try (...) header uses a resource variable
>>>> declared outside the 'try'? Answer: no exception can be thrown,
>>>> whereas the declaration of a resource variable by 'try' includes an
>>>> initializer expression that can throw an exception. There should be a
>>>> test to "prove" that try (r1) {...} doesn't throw exceptions from
>>>> resource initialization, and that try (r1; AutoCloseable r2 = ...)
>>>> {...} only throws exceptions due to r2's initialization.
>>>>
>>>> Alex
>>>>
>>>> On 10/14/2015 12:12 AM, Sergei Pikalev wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> This is the code with new Project Coin tests from SQE.
>>>>>
>>>>> A webrev with the tests is here:
>>>>>
>>>>> http://cr.openjdk.java.net/~shurailine/webrev.8080641/
>>>>>
>>>>> Please review.
>>>>>
>>>>> Thanks,
>>>>>      Sergei
>>>>>
>>>
>