RFR: 8372248: GTest istream.coverage depends on istream.basic [v4]

Kim Barrett kbarrett at openjdk.org
Wed Feb 11 17:49:55 UTC 2026


On Wed, 11 Feb 2026 12:50:41 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> These two GTests have a strong dependency that `istream.coverage` is ran after `istream.basic`. This goes against the intended design of GTests. (They should be independent).
>> 
>> As such I propose we merge this into one test.
>> 
>> I kept the two `VERBOSE` variables separate. However I am not sure I understand their purpose.
>> Currently changing `VERBOSE_TEST` to `true` will cause the test to fail, (not all cases are covered). And the value have of `VERBOSE_COVERAGE` has no effect. What is observed:
>>  * `(VERBOSE_TEST: false, VERBOSE_COVERAGE: false) -> Success`
>>  * `(VERBOSE_TEST: false, VERBOSE_COVERAGE:  true) -> Success`
>>  * `(VERBOSE_TEST:  true, VERBOSE_COVERAGE: false) -> Failure`
>>  * `(VERBOSE_TEST:  true, VERBOSE_COVERAGE:  true) -> Failure`
>> 
>> But I kept the original behaviour, just merged into one test case rather than two.
>
> Axel Boldt-Christmas has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit:
> 
>   gtest istream.coverage depends on istream.basic

That's some weird looking testing.  But the changes are fine for the issue being
addressed.  Looks good.

test/hotspot/gtest/utilities/test_istream.cpp line 314:

> 312:   istream_coverage_mode(0, cases, total, zeroes);
> 313:   if (cases == 0)  return;
> 314:   if (VERBOSE_COVERAGE || zeroes != 0)

Pre-existing style: missing braces around the then-clause.

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

Marked as reviewed by kbarrett (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28418#pullrequestreview-3786311661
PR Review Comment: https://git.openjdk.org/jdk/pull/28418#discussion_r2794638258


More information about the hotspot-dev mailing list