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

JC Beyler jcbeyler at google.com
Mon Jan 7 18:52:30 UTC 2019


Hi Jini,

It looks good to me but I have a few nits:

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

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?


  77         for (String val: cmdStringList) {
  78             outStringList.add(val);
  79         }


Couldn't we use addAll ?

Thanks,

Jc


On Mon, Jan 7, 2019 at 8:33 AM Jini George <jini.george at oracle.com> wrote:

> Gentle reminder !
>
> Thanks,
> Jini.
>
> On 1/3/2019 11:46 PM, Jini George wrote:
> > Hello!
> >
> > Requesting reviews for:
> >
> > https://bugs.openjdk.java.net/browse/JDK-8215544
> > Webrev: http://cr.openjdk.java.net/~jgeorge/8215544/webrev.03/index.html
> >
> > The changes here are primarily for modifying the ClhsdbLauncher used for
> > SA tests to check if 'sudo' privileges can be added for executing MacOS
> > tests, and if so, adding these privileges before executing the tests.
> > This is related to the changes which were proposed with:
> >
> >
> http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-July/024373.html
> >
> >
> > for (JDK-8199700: SA: Enable jhsdb jtreg tests for Mac OS X).
> >
> > The issue brought out with the proposed changes for JDK-8199700 have
> > been fixed. This patch addresses the case where 'sudo' fails with errors
> > like:
> >
> > sudo: no tty present and no askpass program specified
> >   or
> > sudo: a password is required
> >
> > or if 'sudo' waits for a password. In these cases, the tests continue to
> > get skipped.
> >
> > Many thanks to Goetz Lindenmaier for helping me with multiple rounds of
> > testing of this in the SAP test farms! With this patch, about 22 SA
> > tests get enabled now on MacOS on Mach5.
> >
> > More details at: https://bugs.openjdk.java.net/browse/JDK-8215544
> >
> > Thanks,
> > Jini.
>


-- 

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


More information about the serviceability-dev mailing list