On 11/07/2018 8:00 PM, Jini George wrote:
Thank you, David. My answers inline:

On 7/11/2018 1:54 PM, David Holmes wrote:
Hi Jini,

There are quite a few changes to digest in this - it may have been better to break them up individually:
- sudo use
- refactor to use ClshdbLauncher
- changes to use regex matching

Focusing on the main sudo change the assumption is that on OSX you can run sudo without needing to provide a password - correct? That may be the case in mach5 but I'm not sure how others will go running these tests either in their test farms or locally.
Right -- you would need to provide the password. So it prompts for the password for OSX. (Like how it would have been needed if you had run the test itself with 'sudo'). Examining the /etc/sudoers file to check if no password is needed could have been an option, but that itself would need an sudo, and probably would add unwanted complexity.

So I'm not sure this change is acceptable when it may cause other testing environments to break. At a minimum I'd want to get the opinions of the SAP folk and anyone else doing regular build/test runs.

I'm not sure about the regex changes from contains to matches - won't you need additional wildcards at the start and end of the strings to allow the string to be embedded in a longer string ??

OutputAnalyzer's shouldMatch() uses the find() method of the Matcher class which matches sub-sequences.

Ok.

Thanks,
David

Thanks,
Jini.



Thanks,
David

PS. I start vacation in 48 hours :)

On 11/07/2018 12:38 PM, Jini George wrote:
Gentle reminder !

Thanks,
Jini.

On 7/10/2018 12:14 AM, Jini George wrote:
Requesting reviews for enabling SA tests on OS X for Mach5.

https://bugs.openjdk.java.net/browse/JDK-8199700

Webrev: http://cr.openjdk.java.net/~jgeorge/8199700/webrev.00/

The changes are mostly to include the addition of sudo privileges to the SA launchers for OSX if Platform.shouldSAAttach() fails. Some tests (those using clhsdb) have been refactored to use ClhsdbLauncher for ease of maintainence. This also avoids checks for Platform.shouldSAAttach() for corefile related test cases. More details have been provided in JIRA.

Thanks,
Jini.

Reply via email to