RFR: 8272305: several hotspot runtime/modules don't check exit codes

[Igor Ignatyev]

On Thu, 12 Aug 2021 00:55:57 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Hi all,
>> 
>> could you please review this patch that adds exit code checks to five `runtime/modules` tests?
>> 
>> testing: `runtime/modules` on `{linux,windows,macos}-x64`
>> 
>> Thanks,
>> -- Igor
>
> test/hotspot/jtreg/runtime/modules/IgnoreModulePropertiesTest.java line 61:
> 
>> 59:     // the illegal value and then check that its corresponding property is handled
>> 60:     // correctly.
>> 61:     public static void testOption(boolean positive,
> 
> Can we rename "positive" to "shouldSucceed" please.

I actually started w/ `shouldSucceed`, but it can be kinda misleading as a test succeeds in both cases, it just in one case, it expects JVM to exit normally and in another to hit an error. how about `expectError`?

> test/hotspot/jtreg/runtime/modules/IgnoreModulePropertiesTest.java line 77:
> 
>> 75: 
>> 76:     public static void main(String[] args) throws Exception {
>> 77:         testOption(false, "--add-modules", "java.sqlx", "jdk.module.addmods.0", "java.lang.module.FindException");
> 
> It would be a litle more obvious if you did:
> 
> `final boolean shouldSucceed = true;`
> 
> and then do either:
> 
> `testOption(!shouldSucceed, ...);`
> 
> or
> 
> `testOption(shouldSucceed, ...);`

not sure if `testOption(!shouldSucceed,...)` looks/reads nice, I can do `testOption(/*shouldSucceed=*/true, ...);` instead.

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

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