Hello Alan,
On 01/11/21 1:03 am, Alan Bateman wrote:
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.
I looked around some existing tests and I could find some which have a
"realMain" and a "main" methods. But in all such test cases, the
"realMain" is just called as a method within the same process. So I
guess that's not the one you meant. I'll look around some more to try
and find the expected style for such child processes.
assertTestPrerequisite tests if the modules requires has modifiers so let's rename the method to
make this clearer. Also "requirements" should be "requires".
Fixed in the updated version of the PR.
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
Renamed fromBootLayer to javaSQLModuleFromBootLayer in the updated
version of the PR.
on the java.sql module means the test needs "@modules java.sql"
Done.
Style wise, the over-use of the "final" modifier is annoying, most/all of them
aren't needed.
I removed all the "final" modifiers in the updated version of the PR.
Test continues to pass with these changes.
-Jaikiran
