On Thu, 30 Nov 2023 12:19:33 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Hello Stefan, > > > The test were I want to use this is a long-running stress test that is > > known to be good at shaking out JVM hangs. It has been written to provide > > its own output about spawned processes and what they are doing, and have > > that info written to appropriate files. For me, the OutputAnalyzer logging > > just adds redundant output to the streams where I don't want the > > information. > > Thank you for that context. What you are then proposing is for a way to reuse > this utility class in specific tests, like the one you are working on, > without needing the progress logs. So the idea isn't really to use this new > API in all newly developed tests which use this `ProcessTools` library. Plus > the implementation in this PR, retains the existing behaviour of logging the > progress by default, which is a good thing. Yes, you summarize this well. > > Given that context, I don't have any objection in introducing this > enhancemnet. During future reviews, I think, it wouldn't be too difficult to > catch misuses of this new API in newer tests where the progress logs > shouldn't be disabled. Yes, I agree. It would be quite obvious if someone tries to turn this off. > > As for whether this "config" should be externalized - I think it shouldn't > be, since whether or not the progress logs will be excessive and too > distracting (and thus need to be disabled) would be known at test development > time itself. I agree. > The copyright year on OutputBuffer.java will need an update. Fixed. Thanks for taking a look at this! ------------- PR Comment: https://git.openjdk.org/jdk/pull/16807#issuecomment-1834103958
