On Wed, 25 Oct 2023 16:53:18 GMT, Johannes Bechberger <jbechber...@openjdk.org>
wrote:
>> test/lib/jdk/test/lib/JDWP.java line 60:
>>
>>> 58: if (parts.length != 2) {
>>> 59: return null;
>>> 60: }
>>
>> This is a bit presumptuous in that it assumes that the first 2 token line of
>> output is the line we are looking for. The reason `parseListenAddress()`
>> tries to match a specific pattern is because sometimes tests are run in a
>> way that causes the debuggee to produce some extra output (like logging
>> output) and this output needs to be skipped. At the very least you should
>> be checking for a pattern that includes "dt_socket".
>>
>> BTW, I tried using `printf` instead of `echo` so the debug agent could be
>> made to generate a "Listening for transport..." line, in which case this new
>> `parseLaunchEchoListenAddress()` would not be needed, but I could not get
>> the argument processing right. Something about exec() and printf just
>> doesn't work right.
>
> Making too many not enough assumptions. The other method didn't make
> assumptions on the format of address and connection type, so I thought that I
> shouldn't too. I could create a temporary shell file that mimicks the other
> output though.
`parseListenAddress()` looks for a fairly specific format:
`listenRegexp = Pattern.compile("Listening for transport \\b(.+)\\b at address:
\\b(.+)\\b");`
However, it does not assume the transport type, so maybe doing that here is not
such a good idea. I don't like adding shell scripts to tests. We've been
converting tests from shell scripts for years now because they tend to be
problematic.
How about using something like launch="echo Listen Args: ". then you have a
more specific string you can pattern match on:
`Listen Args: dt_socket 42305`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16358#discussion_r1372079078
