Request for review (XXS): JDK-8004066: TEST_BUG: test/java/lang/Math/DivModTests.java assumes ArithmeticException message

[David Holmes]

Hi Kris,

On 3/12/2012 4:37 PM, Krystal Mo wrote:
> On 12/03/2012 08:47 AM, David Holmes wrote:
>> On 3/12/2012 5:36 AM, Krystal Mo wrote:
>>> Hi Alan and core-libs-dev,
>>>
>>> Thanks for the review. I've taken your advise and updated the webrev:
>>> http://cr.openjdk.java.net/~kmo/8004066/webrev.00/
>>>
>>> Could you please review the updated version?
>>
>> If you no longer check the exception message then there's no need to
>> modify the exception construction.
>>
>
> The exception message passed to the constructor call may be mistaken as
> a part of the test if we leave it there as it is now. There are comments
> on those lines which should be sufficient to show the intent of checking
> division by zero. So I took Alan's advise and made the update. Don't
> have a strong opinion on this one, though; I'm okay either way.
>
>> I can't help but think we've lost something in making this change
>> though - any ArithmeticException will cause the test to pass, when it
>> should only be exceptions from divide-by-zero that are ok. :( I don't
>> see any way around it though.
>>
>
> Well, we've lost something that wasn't there in the first place. There's
> no good way of verifying whether an ArithmeticException was thrown from
> a division by zero or not.

Well the exception message indicates it for hotspot - and that is what 
the test was checking for. Now it is a VM neutral test but isn't testing 
what we would like. AS I said I don't see a way around it.

David
-----

  Having just line numbers in the exception
> stack trace doesn't help; if it actually kept the bytecode index of the
> exception throw site, there might have been a way to verify if that bci
> was pointing to a idiv/ldiv instruction. But we don't have that
> information in the exception object.
>
> Thanks,
> Kris
>
>> David
>> -----
>>
>>
>>> Also, could anybody from the JDK side sponsor this change, please?
>>>
>>> Thanks,
>>> Kris
>>>
>>> On 12/01/2012 11:54 PM, Alan Bateman wrote:
>>>> On 30/11/2012 18:48, Krystal Mo wrote:
>>>>> Hi all,
>>>>>
>>>>> Could I get a couple of review for this small change, please?
>>>>> And could someone from the JDK side sponsor this change?
>>>>>
>>>>> Bug: https://jbs.oracle.com/bugs/browse/JDK-8004066
>>>>> Webrev: http://cr.openjdk.java.net/~kmo/8004066/webrev.00/
>>>>>
>>>>> The DivModTest introduced in JDK-6282196 [1] checks the contents of
>>>>> the exception message, but that message isn't specified in JavaDoc
>>>>> and thus may vary between VM implementations (or even in the same VM).
>>>> It looks okay to me, I assume testIntFloorDivMod() could also be
>>>> changed to create the ArithmeticException with the no-arg constructor
>>>> as the exception message will no longer be tested.
>>>>
>>>> Just a general point, the tests in the jdk repository are not
>>>> conformance tests and so it is okay (and normal) for these tests to
>>>> exercise highly implementation-specific behavior. They are expected to
>>>> be updated and always in sync with the JDK code. Clearly checking
>>>> exception messages will a bit fragile, more so in this this case as
>>>> the exception messages are coming from the VM.
>>>>
>>>> -Alan.
>>>>
>>>>
>>>
>