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

[Ivan Gerasimov]

Thank you Roger for looking at this.


On 27.02.2014 2:19, roger riggs wrote:
> Hi Ivan,
>
> A few comments on UnixCommands:
>
>  - The trailing "_" on the method names looks very odd, they could all 
> use some suffix "Pgm" "X", etc.
>
I replaced true_() and false_() with explicit calls to findCommand().

>  - I'm not sure the specific command methods do you any good
>    (over a method with a string argument). since they immediately 
> revert to the string command name.
>
They are shorter :-)

>  - The final "0" in findCommand0 is using the convention for a native 
> method but it is not native.
>
Yes. But I think the convention is more general -- the f0() is usually a 
function of a lower level than f().
Grepping shows some examples:
./jdk/share/classes/java/security/KeyStore.java: run0() is called from run()
./jdk/share/classes/java/util/regex/Pattern.java: match0() is called 
from match() and
In the tests, main0() sometimes is called from main(), test0() from 
test(), etc.

No problem to rename it, but I don't think it should confuse too much.

>  - Use the File methods for concatenation rather than string 
> concatenation.
>    i.e. new File (parent, child).

Good point, thanks!

Please find the updated webrev here:
http://cr.openjdk.java.net/~igerasim/6943190/2/webrev/

Sincerely yours,
Ivan

>
> Roger
>
>
>
>
> On 2/26/2014 4:30 PM, Ivan Gerasimov wrote:
>>
>> On 24.02.2014 23:38, Martin Buchholz wrote:
>>> Thanks for working on these ancient Process tests.
>>> I would prefer to see them using "feature tests".
>>>
>>> You wanna do "cat /dev/zero"?  Then look for cat in /bin and 
>>> /usr/bin and check for /dev/zero as well, e.g. using File.isExecutable
>>>
>> OK. Here's another iteration.
>> I introduced a utility class UnixCommands, which encapsulates 
>> searching for the command under several fixed directories.
>>
>> I didn't touch /dev/null though.  I think we can assume it exists in 
>> every Unix system.  Many other tests depend on it anyways.
>>
>> Would you please have a chance to review the updated wevrev?
>> http://cr.openjdk.java.net/~igerasim/6943190/1/webrev/
>>
>> Sincerely yours,
>> Ivan
>>
>>> Doing OS-specific things like:
>>>
>>> +    static final boolean isLinux =
>>> +            System.getProperty("os.name <http://os.name>", 
>>> "unknown").startsWith("Linux");
>>>
>>> should be a last resort.
>>>
>>>
>>> On Mon, Feb 24, 2014 at 9:26 AM, Ivan Gerasimov 
>>> <ivan.gerasimov at oracle.com <mailto:ivan.gerasimov at oracle.com>> wrote:
>>>
>>>     Hello!
>>>
>>>     We've got one quite old report about been unable to run the test
>>>     under Android.
>>>     https://bugs.openjdk.java.net/browse/JDK-6943190
>>>
>>>     That was due to hard-coded path to the cat utility as /bin/cat.
>>>     When investigating, I found two other tests under the same
>>>     directory that use /usr/bin/cat and /usr/bin/echo.
>>>     These two test seem to (almost) never run because of the unusual 
>>> path.
>>>
>>>     Here's the first version of the webrev, with the fix to only three
>>>     tests mentioned above:
>>>     http://cr.openjdk.java.net/~igerasim/6943190/0/webrev/
>>> <http://cr.openjdk.java.net/%7Eigerasim/6943190/0/webrev/>
>>>
>>>     java/lang/Runtime/exec/ directory has also got several other
>>>     tests, which run shell commands.
>>>     Some of them have absolute path and some of them don't.
>>>
>>>     So I have a general question: Why would we ever need the absolute
>>>     path for the commands?
>>>     Why wouldn't we rely on the PATH env variable?
>>>
>>>     Sincerely yours,
>>>     Ivan
>>>
>>>
>>>
>>
>
>
>