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.gerasi...@oracle.com <mailto:ivan.gerasi...@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
