On Fri, 11 Aug 2023 05:49:45 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> On OSX, don't require that sudo be used to launch SA tools if developer mode
>> is enabled. More details are in the CR.
>>
>> Due to this change, the following tests are no longer being skipped if the
>> host has developer mode is enabled. They previously required running as root
>> because if sudo was used some files were created with root ownership and
>> could not be deleted:
>>
>> serviceability/sa/ClhsdbDumpclass.java
>> serviceability/sa/TestClassDump.java
>>
>> I tested on my home system (M1) with developer mode enabled and also
>> disabled. Also tested with CI, which appears to have developer mode disabled
>> on most hosts, but there are a few where it is enabled.
>
> test/lib/jdk/test/lib/SA/SATestUtils.java line 98:
>
>> 96: private static boolean developerModeEnabled() {
>> 97: List<String> cmd = new ArrayList<String>();
>> 98: cmd.add("DevToolsSecurity");
>
> Is this tool likely to be in the path?
It is for my work machine, my home machine, and the CI machines. If not
installed then an exception is thrown. It probably requires that xcode be
installed, but this is already a requirement to run the tests.
> test/lib/jdk/test/lib/SA/SATestUtils.java line 113:
>
>> 111: String err = new String(p.getErrorStream().readAllBytes());
>> 112: System.out.print("DevToolsSecurity stdout: " + out);
>> 113: if (out.equals("")) System.out.println();
>
> This looks odd - why not just an unconditional println?
If it is not "", then `out` will include a newline at the end of the last line,
which means you'd end up with an empty line in the output. I considered the
following:
if (!out.equals("")) {
System.out.print("DevToolsSecurity stdout: " + out);
}
But it prints nothing in the output if it is "", and I that's not what I was
looking for.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15238#discussion_r1291629140
PR Review Comment: https://git.openjdk.org/jdk/pull/15238#discussion_r1291625220
