[Toybox] Test design issue.

[enh]

On Wed, Apr 26, 2017 at 2:41 PM, Rob Landley <rob at landley.net> wrote:

> On 04/25/2017 12:28 PM, enh wrote:
> > note that the existing SKIP_HOST mechanism is very error prone. iirc
> > none of the sed tests actually run because one near the top needs
> > SKIP_HOST and it never gets unset.
>
> I thought that "VARIABLE=value command" worked the same for shell
> functions as it does for external commands? Hmmm...
>
> $ make test_sed > one.txt
> $ TEST_HOST=1 make test_sed > two.txt
> $ diff -u one.txt two.txt | grep '^[+-]'
> ...
> -PASS: sed sed - - twice
> -PASS: sed match \n literal n
> -PASS: sed c empty continuation
> -PASS: sed 'y/a\bc/de\f/'
> +FAIL: sed 'y/a\bc/de\f/'
> -PASS: sed skip start of range
> +FAIL: sed skip start of range
>
> The first three are the ones we skip, the second the host version is
> failing but the target version isn't so possibly they should be
> annotated too. (Or at least have a comment...)
>
> What shell are you using to run this? (Not bash?)


hmm, maybe this got fixed since i last saw it? with TEST_HOST=1 make
test_sed i'm not seeing any "SKIP" output, whereas when i ran in to this i
saw a whole wall of yellow "SKIP" lines.


>
> > SKIP_HOST is also not very intention-revealing. i'd prefer to explicitly
> > distinguish "host tool is just plain broken" from "we deliberately chose
> > to diverge [because ...]".
>
> Which is why I added comments before them, ala:
>
> # This segfaults ubuntu 12.04's sed. No really.
> SKIP_HOST=1 testing 'sed - - twice' 'sed "" - -' "hello\n" "" "hello\n"
>
> But I see I haven't kept up with that. Blah. (Presumably git annotate
> should give a commit comment, and/or let me know what date to look for a
> mailing list entry or blog post. I'll throw it on the todo list.)


exactly: having the skip mechanism require a string would help keep us
honest.


>
> > cases like sh.test are awkward too...
>
> Yeah shellit() tries to compensate a bit but I haven't opened the toysh
> can of worms yet. (Tim Bird asked me what it would take to do toysh, and
> my estimate was 6 months full-time devoted to it, but he hasn't got a
> budget to buy my time from my employer. Meanwhile $DAYJOB is focused on
> winning solar/wind grid integration monitoring contracts. The market
> opportunity for that is likely to explode in about 3 years and we need
> to deploy stuff people can buy before then and be ready to scale up, so
> getting any time to spend on toybox takes a bit of pushing...)
>
> On the bright side I got https://github.com/landley/mkroot a bit more
> fluffed out this week. Once I get all the musl-cross-make targets
> booting, I can coordinate with Rich to get him to tag and ship an actual
> release with binaries you can download (maybe he'll even put the
> mcm-buildall.sh script in _his_ repo instead of mine).
>
> Building kernels with mkroot needs toybox commit 9be140ebb29f so I'll
> have to cut a new release here to coincide with the mkroot release, but
> it's coming up on time for that anyway...
>
> > exponentiation is a bash extension
> > to the more widespread arithmetic extension (which is all the test
> > tests).
>
> Oh toysh is going to need quite possibly a thousand lines of tests. The
> tests I have _now_ are just ones that I had lying around because I was
> poking at something that was doing this...
>

mksh has a thousand line perl script to run 13k lines of tests that look
like

name: xtrace-2
description:
        Check that "set -x" is off during PS4 expansion
stdin:
        f() {
                print -n "(f1:$-)"
                set -x
                print -n "(f2:$-)"
        }
        PS4='[(p:$-)$(f)] '
        print "(o0:$-)"
        set -x -o inherit-xtrace
        print "(o1:$-)"
        set +x
        print "(o2:$-)"
expected-stdout:
        (o0:sh)
        (o1:shx)
        (o2:sh)
expected-stderr:
        [(p:sh)(f1:sh)(f2:sh)] print '(o1:shx)'
        [(p:sh)(f1:sh)(f2:sh)] set +x



> I thought it was bc/dc but:
>
>   $ dc "1+2"
>   dc: Could not open file 1+2
>   $ bc "1+2"
>   File 1+2 is unavailable.
>
> Those are both _way_ too stupid to use as a quick and dirty calculator
> from the command line. (I usually do echo $((1+2)) when I don't need
> floating point, and fire up python when I do. But there should be a
> better way, possibly I should have toysh $(()) support floating point?
> Hmmm...)
>
> > so although it's "sh.test", it explicitly invokes "bash".
>
> Because toysh should provide the bash alias along with bash functionality.
>
> Ubuntu's epically stupid https://wiki.ubuntu.com/DashAsBinSh broke the
> #!/bin/sh symlink to point to a broken shell (seriously, at the time it
> segfaulted and got signal handling wrong and...), so scripts explicitly
> say #!/bin/bash instead. I intend to provide a reasonable bash
> replacement that runs those scripts.
>
> Bash was the first program linux ever ran (and running bash was the
> reason Linus implemented system calls in his term program), so it was
> the standard shell of Linux from 1991 to 2006. Ubuntu's stated reason
> for changing this (speeding up init scripts) didn't work (they then
> implemented upstart to get parallelism), but they never admitted the
> mistake and reverted it. They said that changing the #! at the start of
> each init script was too intrusive a change, so they broke the kernel
> build instead.
>
> That's why all my scripts say #!/bin/bash and not #!/bin/sh. I need to
> replace _that_.
>
> > and if you "fix" it by running sh, bash-as-sh fails too. for now i just
> have an
> > explicit case to skip sh.test in my Android toybox test runner, but in
> > an ideal world it would be more fine-grained than that.
>
> My infrastructure checks .config and should skip tests for commands that
> are disabled?
>

huh. i hadn't thought of that (though i had thought of its moral
equivalent: checking to see whether the tool in question is a symlink to
`toybox`). that's an option, though i'm potentially still interested in
running the toybox tests against a non-toybox implementation. (but maybe my
runner should check against .config and explicitly say when we're not
running toybox.)


> Alas, I don't have a "pending" for the tests directory. In an ideal
> world I'd be making progress fast enough this wouldn't be an issue. In
> practice we're coming up on the one year anniversary of $DAYJOB's
> "funding knothole" where we've been in perpetual crisis mode ever since.
> (It's compelling technology I really want to see ship, but the pay sucks
> and it's eating my life. Doing what I can here...)
>
> In the meantime, I can add a check for "bash" in the $PATH and skip if
> not". Does that help?


if you hadn't committed it already i'd say "don't worry about it": there's
not enough in sh.test at the moment to worry about. what i _ought_ to be
doing for shell testing is building a perl binary to run the mksh suite,
but the thought of building perl in 2017 really stretches my pragmatism to
its limit!

i'm more interested in seeing the gzip/gunzip/zcat tests go in so we can
start arguing about how to upstream my toybox rewrite of the toolbox
zlib-based gzip/gunzip/zcat. (i did the port on monday, so i do have a
toybox gzip/gunzip/zcat that passes the tests, but i didn't even try to
integrate the half-finished gz code from toys/pending/compress.c.)


> > here though, is there a strong reason to prefer \e? otherwise why not
> > just do what the host does in this case?
>
> Because "man echo" says \e and my escape/unescape plumbing is
> symmetrical. I'd have to special case it to _not_ understand this.
>
> I suppose I can also have the test run the output through sed to replace
> "\e" with "\033" and then it'd except either?
>
> Rob
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net
>



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20170426/029d3121/attachment-0003.htm>