On Tue, 5 Sep 2023 18:05:34 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> I have created an alternative that uses enums to force the user to make a >> decision: >> https://github.com/openjdk/jdk/compare/master...lkorinth:jdk:+process_tools >> . Another alternative is to do the same but instead using an enum (I think >> it is not as good). A third alternative is to use the current pull request >> with a better name. >> >> What do you prefer? Do you have a better alternative? Do someone still think >> the current code is good? I think what we have today is inferior to all >> these improvements, and I would like to make it harder to develop bad test >> cases. > >> What do you prefer? Do you have a better alternative? Do someone still think >> the current code is good? I think what we have today is inferior to all >> these improvements, and I would like to make it harder to develop bad test ca > > The current API (name) is fine and fit for purpose; it does not promise or > hide extra functionality under a simple name. > > There needs to be an explicit intention in the test(s) to support after the > fact that arbitrary flags can be added. > @AlanBateman's proposal for naming > [above](https://github.com/openjdk/jdk/pull/15452#issuecomment-1700459277) > (or similar) would capture more clearly that test options are propagated to > the child process. > Every test writer should be aware that additional command line options may be > mixed in. > > There are many cases in which the ProcessTools APIs are not used to create > child processes and do not need to be used in writing tests. They provide > some convenience but also add a dependency and another API layer to work > through in the case of failures. > > As far as I'm aware, there is no general guidance or design pattern outside > of hotspot tests to propagate flags or use ProcessTools. Adding that as a > requirement will need a different level of communication and change. @RogerRiggs You seem to know what you want w.r.t. the extra java doc comments. Could you help write those? Could we also do that as a separate RFE? I think that would make it easier to get this PR and the javadoc update through the door. ------------- PR Comment: https://git.openjdk.org/jdk/pull/15452#issuecomment-1778669353
