On Mon, 14 Dec 2020 23:58:43 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> Hi, >> >> Please review the following small fix for test >> RemovingUnixDomainSocketTest.java. As explained in the bug comments, the >> issue is due to having two different StreamPumper objects consuming from the >> same stderr, one created by ProcessTools.startProcess() and another by >> OutputAnalyzer(). In the failing cases the StreamPumper processing thread >> created in ProcessTools.startProcess() consumes the first part of the >> deprecation message while the one created in OutputAnalyzer consumes the >> rest. Since out.getStderr() is not empty and does not contain the string "VM >> warning:", the test fails. >> >> I simply replaced the ProcessTools.startProcess() call by a call to start() >> on the ProcessBuilder object, which doesn't use StreamPumper. I added >> stderrShouldBeEmptyIgnoreDeprecatedWarnings(), since as mentioned in 8248162 >> we might not want to hide all warning messages. >> >> Without the patch I can consistently reproduce the issue. With the patch the >> test always passes. >> >> Thanks, >> Patricio > > Changes requested by amenkov (Reviewer). > _Mailing list message from [David Holmes](mailto:david.hol...@oracle.com) on > [serviceability-dev](mailto:serviceability-dev@openjdk.java.net):_ > > On 15/12/2020 10:01 am, Alex Menkov wrote: > > > On Fri, 11 Dec 2020 17:25:33 GMT, Patricio Chilano Mateo <pchilanomate at > > openjdk.org> wrote: > > > Hi, > > > Please review the following small fix for test > > > RemovingUnixDomainSocketTest.java. As explained in the bug comments, the > > > issue is due to having two different StreamPumper objects consuming from > > > the same stderr, one created by ProcessTools.startProcess() and another > > > by OutputAnalyzer(). In the failing cases the StreamPumper processing > > > thread created in ProcessTools.startProcess() consumes the first part of > > > the deprecation message while the one created in OutputAnalyzer consumes > > > the rest. Since out.getStderr() is not empty and does not contain the > > > string "VM warning:", the test fails. > > > I simply replaced the ProcessTools.startProcess() call by a call to > > > start() on the ProcessBuilder object, which doesn't use StreamPumper. I > > > added stderrShouldBeEmptyIgnoreDeprecatedWarnings(), since as mentioned > > > in 8248162 we might not want to hide all warning messages. > > > Without the patch I can consistently reproduce the issue. With the patch > > > the test always passes. > > > Thanks, > > > Patricio > > > > > > Changes requested by amenkov (Reviewer). > > test/hotspot/jtreg/serviceability/attach/RemovingUnixDomainSocketTest.java > > line 70: > > > 68: > > > 69: out.shouldHaveExitValue(0); > > > 70: out.stderrShouldBeEmptyIgnoreDeprecatedWarnings(); > > > > > > Looked at the fix one more time. > > I suppose this new method is not needed as a the message about deprecation > > is VM warning and should be handled by stderrShouldBeEmptyIgnoreVMWarnings. > > Also this new method will fails if jcmd prints some other VM warning > > I think that is the point of the new method. We know deprecation > warnings are harmless and can be ignored. But other warnings may > indicate an issue that needs investigating. > > Cheers, > David Right, as David pointed out the purpose is to only ignore deprecation warnings. Patricio ------------- PR: https://git.openjdk.java.net/jdk/pull/1749
