Hi Sharath,
I think you and Jini need to coordinate your current proposed changes. :)
I have a few comments.
On 30/10/2017 2:29 PM, Sharath Ballal wrote:
Hello,
This webrev has changes for a framework for running the ‘jhsdb clhsdb’
commands and verifying the output. It also has sanity tests for 8 of
I can't help but think the launcher should be able to utilize
OutputAnalyzer. ??
Do you require the input commands to include the "quit"? Is there a
reason the launcher couldn't handle that itself?
Can the launcher internalize the management of the LingeredApp?
Can the launcher also handle the shouldSAAttach check?
I can imagine the test logic reducing to a very simple:
println(Starting test of ...
List<String> cmds = List.of( ...);
List<String> expected = List.of(...);
List<String> unexpected = Listd.of(...);
ClhsdbLauncher.run(cmds, expected, unexpected); // static method
println("test PASSED");
I don't see why the test classes should be subclasses of the
Clhsdblauncher class - the tests are not launchers themselves. The
launcher class is just a utility class that should have public rather
than protected methods.
I'm not sure the approach of sending a set of commands, and having a set
of expected outputs is the right/best way to test this. I would expect
to see a check of the expected outcome for each command ie send a
command, check the result, send the next command, etc. Or if you can
put/detect delimiters in the output you could still send all the
commands and then bulk process the output. But the present approach
allows for the commands to actually do the wrong things, as long as the
expected output appears somewhere.
It also unclear what output corresponds to what command and why. Eg in
the longConstant test it is not obvious why you expect to see
markOopDesc::epoch_mask_in_place, or the difference in expected output
between:
57 "longConstant jtreg::test 6\n",
58 "longConstant jtreg::test\n",
I'm guessing the first silently declares and sets a variable, while the
second reads it back - is that right?
Thanks,
David
the clhsdb commands.
Pls review the changes.
Bug ID: https://bugs.openjdk.java.net/browse/JDK-8190198
Webrev: http://cr.openjdk.java.net/~sballal/8190198/webrev.00/
Thanks,
Sharath
