[7u6] Review request for 7131021: [macosx] Consider using system properties to pass arguments from the launcher to AWT/SplashScreen

[Anthony Petrov]

Hi Kumar and Mike,

Thanks for the review!

Initially I wanted to invoke the sub-tests from Java, too, but the 
Runtime functionality won't let me redirect input/output/error streams 
from a child process, so I ended up with a shell test.

Given that launcher tests already employ the ProcessBuilder, and thus 
deal with IO streams redirection, I like the idea of using it for this 
test as well. The changes suggested by Kumar look great. Thanks!

I've also incorporated feedback from Mike and added a line to the 
comments that explains the necessity for the _<pid> part.

Here's the updated webrev:

http://cr.openjdk.java.net/~anthony/7u6-2-keepEnvVars-7131021.1/

--
best regards,
Anthony

On 4/11/2012 7:16 AM, Kumar Srinivasan wrote:
> Hi Anthony,
> 
> I personally don't want to see shell based test, I spent
> several hours cleaning up the shell tests in the launcher area
> for jdk8, there are few left, I will get to them sooner
> or later. I am assuming this will be forward ported to jdk8.
> 
> Having said that, I have done the following, which simplifies
> a great deal.
> 
> 1. Removed the shell script
> 2. Piggy back the test control onto TestSpecialArgs.java
>    which was introduced for MacOSX, anyway.
> 3. Java invoking java exposed an unique problem where
>    we have multiple env variables
> ex:
> JAVA_MAIN_CLASS_ABC=com.sun.javatest.regtest.Main
> JAVA_MAIN_CLASS_XYZ=EnvironmentVariables
> 
>    To fix the above, TestHelper needed some adjustments.
> 
> 
> It was simpler to show you the changes in a webrev. :)
> Full webrev is here:
> http://cr.openjdk.java.net/~ksrini/7131021/webrev/
> 
> The incremental changes wrt. your changes are here:
> http://cr.openjdk.java.net/~ksrini/7131021/webrev/webrev.delta/index.html
> 
> I have tested the above changes on Mac and Solaris.
> 
> Thanks
> Kumar
> 
> ----- anthony.petrov at oracle.com wrote:
> 
>> From: anthony.petrov at oracle.com
>> To: macosx-port-dev at openjdk.java.net, kumar.x.srinivasan at oracle.COM
>> Sent: Tuesday, April 10, 2012 10:23:19 AM GMT -08:00 US/Canada Pacific
>> Subject: [7u6] Review request for 7131021: [macosx] Consider using system properties to pass arguments from the
>> launcher to AWT/SplashScreen
>>
>> Hello,
>>
>> Please review a fix for http://bugs.sun.com/view_bug.do?bug_id=7131021
>> at:
>>
>> http://cr.openjdk.java.net/~anthony/7u6-2-keepEnvVars-7131021.0/
>>
>> The comments added to java_md_macosx.c have been approved by CCC 
>> already. This is mainly a review for the newly added test that checks
>>
>> for the environment variables that can be set by the launcher on the
>> Mac.
>>
>> A short background to avoid any confusion regarding the synopsis of
>> the 
>> bug. We've discussed the issue thoroughly and come to a conclusion
>> that 
>> we want to preserve the existing behavior, i.e. use the environment 
>> variables to pass some information from the launcher to AWT. With this
>>
>> fix we add comments to the source code to make sure maintainers of the
>>
>> code are aware of compatibility issues related to the variables,
>> should 
>> they need to modify this code. Also, we're adding an automatic 
>> regression test that verifies that the launcher correctly sets these 
>> variables.
>>
>> --
>> best regards,
>> Anthony