RFR: 8313357: Revisit requiring SA tests on OSX to either run as root or use sudo

[Chris Plummer]

On Fri, 11 Aug 2023 05:49:45 GMT, David Holmes <dholmes at 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