RFR: 8252148: vmError::controlled_crash should be #ifdef ASSERT [v3]

Coleen Phillimore coleenp at openjdk.java.net
Fri Dec 11 20:50:01 UTC 2020


On Fri, 11 Dec 2020 20:22:15 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> I think it's better to fix JDK-8257229 separately, so it's easier to track the issues.
>> 
>> Adding `.*` here affects all tests, and it might make the match more liberal than necessary. E.g., if we have `msg = "end of the world"`, the regexp would inadvertently match
>> 
>> assert failed: some unrelated assert
>> Whew, we avoided the end of the world
>> 
>> It would be better to add the .* in a specific test like 
>> 
>> TEST_VM_ASSERT_MSG(vmErrorTest, assert1, ".*assert.str == NULL. failed: expected null") {
>
> A better example for not adding the `.*` :  `TEST_VM_ASSERT_MSG(...., "a == true")` would incorrectly match `assert failed: !a == true`

I reviewed your change.  There are 19 pre-existing tests that only want to test the message, which seems legitimate.

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

PR: https://git.openjdk.java.net/jdk/pull/1723


More information about the hotspot-dev mailing list