[Toybox] avoiding testing shell builtins

[enh]

this also fixed the echo test failure. now at 68 passed, 11 failed
(though that includes the split test failure that's really just "mksh
doesn't support that bash extension").

On Wed, Jun 26, 2019 at 10:39 PM enh <enh at google.com> wrote:
>
> On Wed, Jun 26, 2019 at 10:33 PM enh <enh at google.com> wrote:
> >
> > On Wed, Jun 26, 2019 at 9:45 PM Rob Landley <rob at landley.net> wrote:
> > >
> > > 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...)
> >
> > hmm. i already _do_ run each test independently (and in a relatively
> > clean environment)... and that's the problem here. i did this to
> > myself:
> >
> >   adb shell -t "export FILES=/data/local/tmp/toybox-tests/tests/files/ ; \
> >                 export VERBOSE=1 ; \
> >                 export CMDNAME=$toy; \
> >                 export C=$toy; \
> >                 export LANG=en_US.UTF-8; \
> >                 mkdir $tmp_dir/$toy && cd $tmp_dir/$toy ; \
> >                 source /data/local/tmp/toybox-tests/runtest.sh ; \
> >                 source /data/local/tmp/toybox-tests/tests/$toy.test ; \
> >                 if [ "\$FAILCOUNT" -ne 0 ]; then exit 1; fi; \
> >                 cd .. && rm -rf $toy"
> >
> > fixed by
> >
> > -                export C=$toy; \
> > +                export C=\"$(which $toy)\"; \
>
> well, okay, maybe there should be a \ before the first $ there :-)
>
> > in my runner. sorry about that!
> >
> > > 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...)
> >
> > the sed loop test hangs with TEST_HOST=1 for you too now? haven't we
> > had this before where i reported that when google switched from ubuntu
> > to debian?
> >
> > > 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
> > > >     >
> > > >