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

[Krystal Mo]

Hi David,

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. 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.
>>>
>>>
>>