On Fri, 29 Oct 2021 04:19:21 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

> Do you mean the jtreg `driver` style this test is using? I used it because it 
> was necessary to compare output (the hashCode value) between multiple JVM 
> runs. I use the `driver` along with the `ProcessBuilder` to capture the 
> output between multiple JVM runs and compare those.

No, I mean the test needs cleanup so it can easily be maintained. For starters 
I think the main method can take a parameter to indicate its the child process. 
HashCodeChecker.main becomes a method in ModuleDescriptorHashCodeTest.  You'll 
see several examples of this in the test suite.

assertTestPrerequisite tests if the modules requires has modifiers so let's 
rename the method to make this clearer. Also "requirements" should be 
"requires". 

fromBootLayer returns the "java.sql" module so should be renamed to something 
clearer, or changed to take a parameter with the module name.  The dependency 
on the java.sql module means the test needs "@modules java.sql"

Style wise, the over-use of the "final" modifier is annoying, most/all of them 
aren't needed.

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

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

Reply via email to