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