RFR: JDK-8215544: SA: Modify ClhsdbLauncher to add sudo privileges to enable MacOS tests on Mach5

[JC Beyler]

Hi Jini,

Looks good to me now :) Thanks for handling the comments :)
Jc


On Tue, Jan 8, 2019 at 8:11 PM Jini George <jini.george at oracle.com> wrote:

> Thank you very much for the review, JC. My comments inline:
>
> On 1/8/2019 12:22 AM, JC Beyler wrote:
> >
> >
> http://cr.openjdk.java.net/~jgeorge/8215544/webrev.03/test/hotspot/jtreg/serviceability/sa/ClhsdbLauncher.java.udiff.html
> >    +                // by appending 'sudo' to it. Check now if sudo
> > passes in this
> >    +                // environment with a simple 'echo' command.
> >       -> Really shouldn't provide implementation details about what is
> > being done by that method; if we change that, this comment will rot :-)
> > canAddPrivileges is explicit enough in my mind
>
> Done. I have removed the comment.
>
> >
> >
> http://cr.openjdk.java.net/~jgeorge/8215544/webrev.03/test/lib/jdk/test/lib/SA/SATestUtils.java.html
> >
> >      -> You put some System.out.println; are they intended to still be
> > there or were they for debug?
>
> I guess you meant this one:
> System.out.println("Adding 'sudo -E' to the command.");
>
> This one is meant to be there.
>
> >
> >
> >    77         for (String val: cmdStringList) {
> >    78             outStringList.add(val);
> >    79         }
> >
> >
> > Couldn't we use addAll ?
>
> Done.
> The modified webrev is at:
>
> http://cr.openjdk.java.net/~jgeorge/8215544/webrev.04/index.html
>
> Thanks,
> Jini.
>
>
>
>

-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190110/e5360b2d/attachment.html>