[Toybox] avoiding testing shell builtins

[Rob Landley]

On 6/26/19 6:49 PM, enh wrote:
> No, this is on Android. I get an error from /system/bin/mksh from the first
> test.

I mean when there _is_ a shell builtin, TEST_HOST should _not_ test it. This
would be a design change. Is this the correct behavior? I'd rather not special
case this, I want to figure out what the right semantics are so they can be
consistent and documented.

Hmmm... I think the right semantics are that TEST_HOST shouldn't test the shell
builtin _if_ there's a version in the $PATH. So call which and see if it returns
something, and if it doesn't use the pathless version...

Something like:

--- a/scripts/test.sh
+++ b/scripts/test.sh
@@ -34,11 +34,13 @@ do_test()
   cd "$TESTDIR" && rm -rf testdir && mkdir testdir && cd testdir || exit 1
   CMDNAME="${1##*/}"
   CMDNAME="${CMDNAME%.test}"
-  C="$CMDNAME"
   if [ -z "$TEST_HOST" ]
   then
     C="$TESTDIR/$CMDNAME"
     [ ! -e "$C" ] && echo "$CMDNAME disabled" && return
+  else
+    C="$(which $CMDNAME 2>/dev/null)"
+    [ -z "$C" ] && "C=$CMDNAME"
   fi

   . "$1"

Which is hard to test because what do we have a toybox command for that_doesn't_
have $PATH entry in debian but has a shell builtin... ah, ulimit. Except I
haven't written a ulimit.test yet because todo item...

This todo item is "I shouldn't source tests, I should run them as indpendent
processes so they don't have to clean up their environment", but that means
making sure I'm exporting the variables I set and auditing all the tests. (Not a
hard todo item, just time consuming...)

Anyway, not a regression, so I checked it in.

(Well I say it's not a regression. Why are two sed host tests failing now? Is
this an ubuntu->devuan thing? Sigh...)

Rob


> On Wed, Jun 26, 2019, 16:42 Rob Landley <rob at landley.net
> <mailto:rob at landley.net>> wrote:
> 
>     On 6/26/19 3:42 PM, enh wrote:
>     > your patch doesn't fix this for me. even with testcmd i'm still seeing
>     > the mksh builtin kill get run rather than the toybox kill...
> 
>     With TEST_HOST it tests just the command name, however the shell decides to
>     resolve that. Without that it tests the path to the toybox command.
> 
>     Instead you want the TEST_HOST version also to not test the shell builtin? (Some
>     host systems don't have non-builtin versions of things like "echo". It's a
>     design question of what counts as the "host version"...)
> 
>     Rob
> 
> 
>     > On Sat, Jun 22, 2019 at 7:35 PM Rob Landley <rob at landley.net
>     <mailto:rob at landley.net>> wrote:
>     >>
>     >> On 6/22/19 11:26 AM, enh via Toybox wrote:
>     >>> i realized today that the kill tests are testing the shell's builtin
>     >>> kill, not the toybox kill. one solution would be to use env, but
>     >>> presumably that would be better in the infrastructure rather than
>     >>> having to remember to do it in each test that might be subverted by a
>     >>> builtin?
>     >>
>     >> This is why I added testcmd and $C a while back. (Although currently it won't
>     >> work if the path to the testing directory has a space in it, because I hadn't
>     >> quoted $C. Just fixed that...)
>     >>
>     >> Rob
>     >
>