On Mon, 23 Mar 2026 22:15:56 GMT, Phil Race <[email protected]> wrote:

>> This fix updates DataBuffer subclasses to actually adhere to their stated 
>> specifications by rejecting certain invalid parameters for constructors and 
>> getters and setters.
>> A new egression test for each of the constructor and getter/setter cases is 
>> supplied.
>> 
>> No existing regression tests fail with this change, and standard demos work.
>> 
>> Problems caused by these changes are most likely to occur if the client has 
>> a bug such that 
>> - a client uses the constructors that accept an array and then supplies a 
>> "size" that is greater than the array.
>> - a client uses the constructors that accept an array and then supplies a 
>> "size" that is less than the array and then uses getter/setters that are 
>> within the array but outside the range specified by size. 
>> 
>> Since very few clients (and just one case in the JDK that I found) even use 
>> these array constructors the changes are unlikely to make a difference to 
>> clients.
>> 
>> The CSR is ready for review https://bugs.openjdk.org/browse/JDK-8378116
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8377568

Do you mind using JUnit for testing? jtreg supports running JUnit tests. Such 
unit tests could be much cleaner and *verify all the conditions* where 
exceptions are expected, including all the edge cases.

The test report clearly shows which test failed, there's no need to scan the 
test log to determine it; running JUnit tests in an IDE provides even better 
experience.

I created [a PoC for such a test 
suite](https://github.com/aivanov-jdk/jdk/commit/c27deaaa3f911554b3ba3ff784385b271a432767).
 An abstract `DataBufferTest` defines a set of tests and abstract methods to 
call constructors. When an exception is thrown, it validates not only the 
correct class but the expected message too.

The test classes, `DataBufferByteTest` and `DataBufferIntTest` can be with 
jtreg as regular tests with `@run main` by passing the path to the test.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/29766#issuecomment-4226381231

Reply via email to