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

[David Holmes]

On Thu, 12 Aug 2021 17:37:29 GMT, Igor Ignatyev <iignatyev at openjdk.org> wrote:

>> 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`?

Both "error" and "succeed" have the same problem if you look at it from the perspective of the test rather than the state of the VM being exec'd. How about shouldExitNormally or expectVMFail, or something that makes it VM specific?

>> 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.

Okay

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

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