RFR [6943190] TEST_BUG: java/lang/Runtime/exec/ExecWithInput.java hardcodes path to cat

Ivan Gerasimov ivan.gerasimov at oracle.com
Sun Mar 16 16:52:54 UTC 2014


Thank you Martin, for your comments!

On 16.03.2014 4:41, Martin Buchholz wrote:
> Thanks for trying to make these tests more robust.
>
>    33     private static final String[] paths = {"/bin", "/usr/bin",
>    34             "/usr/local/bin", "/system/bin", "/sbin", "/usr/bin/sbin",
>    35             "/usr/local/sbin", "/system/sbin"};
>
> I think in practice { "/bin", "/usr/bin" } would work very well.  I 
> would definitely leave out /usr/local, since that is a location for 
> non-standard program variants to be installed.  I don't know anything 
> about /system - I would leave it out.  Pedants would suggest getting 
> the system default PATH using "getconf PATH", and that might get you 
> /usr/xpg4/bin, but probably overkill here.
>
I agree, and I'll make this search list shorter.

> I wouldn't fall back to looking on PATH.  There should be as few 
> dependencies on the user's environment as possible.
>
I imagine the perfect world with this information provided to the test 
by the harness tool.
These commands are executed from shell scripts too, and I've seen the 
same problem with hard-coded path.
Take https://bugs.openjdk.java.net/browse/JDK-6543375, for example.
The fix there was to remove the absolute path and rely on the PATH variable.


> I would leave out the crufty windows-checking code from each test, and 
> instead write test code like:
>
> If (UnixCommands.tee() == null || UnixCommands.cat() == null)
>   return;
>

Done. I kept the check for specific OS, where it is required by the test 
itself.

Would you please take a look at the updated webrev with your suggestions 
incorporated:
http://cr.openjdk.java.net/~igerasim/6943190/3/webrev/


Sincerely yours,
Ivan


> Imagine these tests will be run on some other strange OS someday.
>
>
>
> On Fri, Mar 14, 2014 at 9:58 AM, Ivan Gerasimov 
> <ivan.gerasimov at oracle.com <mailto:ivan.gerasimov at oracle.com>> wrote:
>
>     Hello!
>
>     This is a friendly reminder.
>     Can someone with the Reviewer status please help review this?
>
>     In short: we have a few java tests that run shell commands.
>     In some tests the path to the command is omitted, in other tests
>     an absolute path is given.
>     In the third group of tests, the *other* absolute path to the same
>     commands is used.
>
>     As Martin suggested, I added searching for the command at certain
>     fixed directories before using it.
>
>     The final webrev can be found here:
>     http://cr.openjdk.java.net/~igerasim/6943190/2/webrev/
>     <http://cr.openjdk.java.net/%7Eigerasim/6943190/2/webrev/>
>
>     Thanks in advance,
>     Ivan
>
>
>




More information about the core-libs-dev mailing list